New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New: no-restricted-properties rule (fixes #3218) #7017
Changes from 2 commits
1e849a8
0a548a8
fc32de5
a938c40
9e91326
576e4cc
c7e2ca2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
# Disallow certain object properties (no-restricted-properties) | ||
|
||
Certain properties on objects may be disallowed in a codebase. This is useful for deprecating an API or restricting usage of a module's methods. This rule looks for accessing a given property key on a given object name, either when reading the property's value or invoking it as a function. You may specify an optional message to indicate an alternative API or a reason for the restriction. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you expand on this? This section is supposed to be about the concept that the rule is based on rather than about the rule itself. Explain why you'd want to restrict properties and maybe give an example ("window.location"? Something else?). The parts about the rule should go after "Rule Details heading. |
||
|
||
## Rule Details | ||
|
||
This rule is aimed at disallowing certain object properties from your code. | ||
|
||
### Options | ||
|
||
This rule takes a list of objects, where the object name and property names are specified: | ||
|
||
```json | ||
{ | ||
"rules": { | ||
"no-restricted-properties": [2, [{ | ||
"object": "disallowedObjectName", | ||
"property": "disallowedPropertyName" | ||
}]] | ||
} | ||
} | ||
``` | ||
|
||
Multiple object/property values can be disallowed, and you can specify an optional message: | ||
|
||
```json | ||
{ | ||
"rules": { | ||
"no-restricted-properties": [2, [{ | ||
"object": "disallowedObjectName", | ||
"property": "disallowedPropertyName" | ||
}, { | ||
"object": "disallowedObjectName", | ||
"property": "anotherDisallowedPropertyName", | ||
"message": "Please use allowedObjectName.allowedPropertyName." | ||
}]] | ||
} | ||
} | ||
``` | ||
|
||
The following patterns are considered problems: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Our format is now
|
||
|
||
```js | ||
/* eslint no-restricted-properties: [2, { | ||
"object": "disallowedObjectName", | ||
"property": "disallowedPropertyName" | ||
}] */ | ||
|
||
var example = disallowedObjectName.disallowedPropertyName; /*error Disallowed object property: disallowedObjectName.disallowedPropertyName.*/ | ||
|
||
disallowedObjectName.disallowedPropertyName(); /*error Disallowed object property: disallowedObjectName.disallowedPropertyName.*/ | ||
``` | ||
|
||
The following patterns are not considered problems: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
```js | ||
/* eslint no-restricted-properties: [2, { | ||
"object": "disallowedObjectName", | ||
"property": "disallowedPropertyName" | ||
}] */ | ||
|
||
var example = disallowedObjectName.somePropertyName; | ||
|
||
allowedObjectName.disallowedPropertyName(); | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
If you don't have any object/property combinations to restrict, you should not use this rule. | ||
|
||
## Related Rules | ||
|
||
* [no-restricted-syntax](no-restricted-syntax.md) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
/** | ||
* @fileoverview Rule to disallow certain object properties | ||
* @author Will Klein & Eli White | ||
*/ | ||
|
||
"use strict"; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Rule Definition | ||
//------------------------------------------------------------------------------ | ||
|
||
module.exports = { | ||
meta: { | ||
docs: { | ||
description: "disallow certain properties on certain objects", | ||
category: "Node.js and CommonJS", | ||
recommended: false | ||
}, | ||
|
||
schema: { | ||
type: "array", | ||
items: { | ||
type: "object", | ||
properties: { | ||
object: { | ||
type: "string" | ||
}, | ||
property: { | ||
type: "string" | ||
}, | ||
message: { | ||
type: "string" | ||
} | ||
}, | ||
additionalProperties: false, | ||
required: [ | ||
"object", | ||
"property" | ||
] | ||
}, | ||
uniqueItems: true | ||
} | ||
}, | ||
|
||
create(context) { | ||
const restrictedCalls = context.options; | ||
|
||
if (restrictedCalls.length === 0) { | ||
return {}; | ||
} | ||
|
||
const restrictedProperties = context.options.reduce(function(restrictions, option) { | ||
const objectName = option.object; | ||
const propertyName = option.property; | ||
|
||
restrictions[objectName] = restrictions[objectName] || {}; | ||
restrictions[objectName][propertyName] = { | ||
message: option.message | ||
}; | ||
|
||
return restrictions; | ||
}, {}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should use /*eslint no-restricted-properties: [error, {object: "obj", property: "foo"}]*/
// should not warn.
toString.toString
obj.toString
obj.valueOf /*eslint no-restricted-properties: [error, {object: "obj", property: "__proto__"}]*/
// should not warn.
obj.toString
obj.valueOf There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, good catch. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has been great. This is the first time I have used |
||
|
||
return { | ||
MemberExpression(node) { | ||
const objectName = node.object && node.object.name; | ||
const propertyName = node.property && node.property.name; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only going to catch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think @mysticatea had recently added a "get simple property name" method On Sep 2, 2016 8:56 AM, "Ilya Volodin" notifications@github.com wrote:
|
||
const matchedObject = restrictedProperties[objectName]; | ||
const matchedObjectProperty = matchedObject && matchedObject[propertyName]; | ||
|
||
if (matchedObjectProperty) { | ||
const message = matchedObjectProperty.message ? " " + matchedObjectProperty.message : ""; | ||
|
||
context.report(node, "'{{objectName}}.{{propertyName}}' is restricted from being used.{{message}}", { | ||
objectName, | ||
propertyName, | ||
message | ||
}); | ||
} | ||
} | ||
}; | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
/** | ||
* @fileoverview Tests for no-restricted-properties rule. | ||
* @author Will Klein | ||
* @copyright 2016 Will Klein. All rights reserved. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove the copyright |
||
*/ | ||
|
||
"use strict"; | ||
|
||
//------------------------------------------------------------------------------ | ||
// Requirements | ||
//------------------------------------------------------------------------------ | ||
|
||
const rule = require("../../../lib/rules/no-restricted-properties"), | ||
RuleTester = require("../../../lib/testers/rule-tester"); | ||
|
||
//------------------------------------------------------------------------------ | ||
// Tests | ||
//------------------------------------------------------------------------------ | ||
|
||
const ruleTester = new RuleTester(); | ||
|
||
ruleTester.run("no-restricted-properties", rule, { | ||
valid: [ | ||
{ | ||
code: "someObject.someProperty", | ||
options: [{ | ||
object: "someObject", | ||
property: "disallowedProperty" | ||
}] | ||
}, { | ||
code: "anotherObject.disallowedProperty", | ||
options: [{ | ||
object: "someObject", | ||
property: "disallowedProperty" | ||
}] | ||
}, { | ||
code: "someObject.someProperty()", | ||
options: [{ | ||
object: "someObject", | ||
property: "disallowedProperty" | ||
}] | ||
}, { | ||
code: "anotherObject.disallowedProperty()", | ||
options: [{ | ||
object: "someObject", | ||
property: "disallowedProperty" | ||
}] | ||
}, { | ||
code: "anotherObject.disallowedProperty()", | ||
options: [{ | ||
object: "someObject", | ||
property: "disallowedProperty", | ||
message: "Please use someObject.allowedProperty instead." | ||
}] | ||
} | ||
], | ||
|
||
invalid: [ | ||
{ | ||
code: "someObject.disallowedProperty", | ||
options: [{ | ||
object: "someObject", | ||
property: "disallowedProperty" | ||
}], | ||
errors: [{ | ||
message: "'someObject.disallowedProperty' is restricted from being used.", | ||
type: "MemberExpression" | ||
}] | ||
}, { | ||
code: "someObject.disallowedProperty", | ||
options: [{ | ||
object: "someObject", | ||
property: "disallowedProperty", | ||
message: "Please use someObject.allowedProperty instead." | ||
}], | ||
errors: [{ | ||
message: "'someObject.disallowedProperty' is restricted from being used. Please use someObject.allowedProperty instead.", | ||
type: "MemberExpression" | ||
}] | ||
}, { | ||
code: "someObject.disallowedProperty; anotherObject.anotherDisallowedProperty()", | ||
options: [{ | ||
object: "someObject", | ||
property: "disallowedProperty" | ||
}, { | ||
object: "anotherObject", | ||
property: "anotherDisallowedProperty" | ||
}], | ||
errors: [{ | ||
message: "'someObject.disallowedProperty' is restricted from being used.", | ||
type: "MemberExpression" | ||
}, { | ||
message: "'anotherObject.anotherDisallowedProperty' is restricted from being used.", | ||
type: "MemberExpression" | ||
}] | ||
} | ||
] | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Lowercase "d" to start the heading (new style)