Skip to content

Commit

Permalink
Chore: enable a modified version of multiline-comment-style on codeba…
Browse files Browse the repository at this point in the history
…se (#9452)

This makes the following changes:

* Creates a project-specific `multiline-comment-style` rule,
  which is the same as the built-in `multiline-comment-style` rule
  except that it ignores the banner comments that we use at the top of
  files.
* Enables the `multiline-comment-style` rule on the ESLint
codebase
  • Loading branch information
not-an-aardvark committed Oct 29, 2017
1 parent cb60285 commit 8e1a095
Show file tree
Hide file tree
Showing 122 changed files with 1,470 additions and 1,039 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.js
Expand Up @@ -24,6 +24,7 @@ module.exports = {
"eslint-plugin/prefer-placeholders": "error",
"eslint-plugin/report-message-format": ["error", "[^a-z].*\\.$"],
"eslint-plugin/test-case-property-ordering": "error",
"eslint-plugin/test-case-shorthand-strings": "error"
"eslint-plugin/test-case-shorthand-strings": "error",
"rulesdir/multiline-comment-style": "error"
}
};
8 changes: 5 additions & 3 deletions Makefile.js
Expand Up @@ -843,9 +843,11 @@ target.checkRuleFiles = function() {
const docText = cat(docFilename);
const idOldAtEndOfTitleRegExp = new RegExp(`^# (.*?) \\(${id}\\)`); // original format
const idNewAtBeginningOfTitleRegExp = new RegExp(`^# ${id}: `); // new format is same as rules index
// 1. Added support for new format.
// 2. Will remove support for old format after all docs files have new format.
// 3. Will remove this check when the main heading is automatically generated from rule metadata.
/*
* 1. Added support for new format.
* 2. Will remove support for old format after all docs files have new format.
* 3. Will remove this check when the main heading is automatically generated from rule metadata.
*/

return idNewAtBeginningOfTitleRegExp.test(docText) || idOldAtEndOfTitleRegExp.test(docText);
}
Expand Down
8 changes: 5 additions & 3 deletions conf/default-cli-options.js
Expand Up @@ -17,9 +17,11 @@ module.exports = {
ignorePath: null,
cache: false,

// in order to honor the cacheFile option if specified
// this option should not have a default value otherwise
// it will always be used
/*
* in order to honor the cacheFile option if specified
* this option should not have a default value otherwise
* it will always be used
*/
cacheLocation: "",
cacheFile: ".eslintcache",
fix: false,
Expand Down
96 changes: 55 additions & 41 deletions lib/ast-utils.js
Expand Up @@ -620,15 +620,17 @@ module.exports = {
node = parent;
break;

// If the upper function is IIFE, checks the destination of the return value.
// e.g.
// obj.foo = (function() {
// // setup...
// return function foo() { ... };
// })();
// obj.foo = (() =>
// function foo() { ... }
// )();
/*
* If the upper function is IIFE, checks the destination of the return value.
* e.g.
* obj.foo = (function() {
* // setup...
* return function foo() { ... };
* })();
* obj.foo = (() =>
* function foo() { ... }
* )();
*/
case "ReturnStatement": {
const func = getUpperFunction(parent);

Expand All @@ -645,23 +647,27 @@ module.exports = {
node = parent.parent;
break;

// e.g.
// var obj = { foo() { ... } };
// var obj = { foo: function() { ... } };
// class A { constructor() { ... } }
// class A { foo() { ... } }
// class A { get foo() { ... } }
// class A { set foo() { ... } }
// class A { static foo() { ... } }
/*
* e.g.
* var obj = { foo() { ... } };
* var obj = { foo: function() { ... } };
* class A { constructor() { ... } }
* class A { foo() { ... } }
* class A { get foo() { ... } }
* class A { set foo() { ... } }
* class A { static foo() { ... } }
*/
case "Property":
case "MethodDefinition":
return parent.value !== node;

// e.g.
// obj.foo = function foo() { ... };
// Foo = function() { ... };
// [obj.foo = function foo() { ... }] = a;
// [Foo = function() { ... }] = a;
/*
* e.g.
* obj.foo = function foo() { ... };
* Foo = function() { ... };
* [obj.foo = function foo() { ... }] = a;
* [Foo = function() { ... }] = a;
*/
case "AssignmentExpression":
case "AssignmentPattern":
if (parent.left.type === "MemberExpression") {
Expand All @@ -676,8 +682,10 @@ module.exports = {
}
return true;

// e.g.
// var Foo = function() { ... };
/*
* e.g.
* var Foo = function() { ... };
*/
case "VariableDeclarator":
return !(
isAnonymous &&
Expand All @@ -686,10 +694,12 @@ module.exports = {
startsWithUpperCase(parent.id.name)
);

// e.g.
// var foo = function foo() { ... }.bind(obj);
// (function foo() { ... }).call(obj);
// (function foo() { ... }).apply(obj, []);
/*
* e.g.
* var foo = function foo() { ... }.bind(obj);
* (function foo() { ... }).call(obj);
* (function foo() { ... }).apply(obj, []);
*/
case "MemberExpression":
return (
parent.object !== node ||
Expand All @@ -700,10 +710,12 @@ module.exports = {
isNullOrUndefined(parent.parent.arguments[0])
);

// e.g.
// Reflect.apply(function() {}, obj, []);
// Array.from([], function() {}, obj);
// list.forEach(function() {}, obj);
/*
* e.g.
* Reflect.apply(function() {}, obj, []);
* Array.from([], function() {}, obj);
* list.forEach(function() {}, obj);
*/
case "CallExpression":
if (isReflectApply(parent.callee)) {
return (
Expand Down Expand Up @@ -930,8 +942,10 @@ module.exports = {
node.type === "FunctionDeclaration" ||
node.type === "FunctionExpression" ||

// Do not check arrow functions with implicit return.
// `() => "use strict";` returns the string `"use strict"`.
/*
* Do not check arrow functions with implicit return.
* `() => "use strict";` returns the string `"use strict"`.
*/
(node.type === "ArrowFunctionExpression" && node.body.type === "BlockStatement")
) {
const statements = node.type === "Program" ? node.body : node.body.body;
Expand All @@ -954,7 +968,7 @@ module.exports = {

/**
* Determines whether this node is a decimal integer literal. If a node is a decimal integer literal, a dot added
after the node will be parsed as a decimal point, rather than a property-access dot.
* after the node will be parsed as a decimal point, rather than a property-access dot.
* @param {ASTNode} node - The node to check.
* @returns {boolean} `true` if this node is a decimal integer.
* @example
Expand Down Expand Up @@ -1183,12 +1197,12 @@ module.exports = {
},

/**
* Gets the parenthesized text of a node. This is similar to sourceCode.getText(node), but it also includes any parentheses
* surrounding the node.
* @param {SourceCode} sourceCode The source code object
* @param {ASTNode} node An expression node
* @returns {string} The text representing the node, with all surrounding parentheses included
*/
* Gets the parenthesized text of a node. This is similar to sourceCode.getText(node), but it also includes any parentheses
* surrounding the node.
* @param {SourceCode} sourceCode The source code object
* @param {ASTNode} node An expression node
* @returns {string} The text representing the node, with all surrounding parentheses included
*/
getParenthesisedText(sourceCode, node) {
let leftToken = sourceCode.getFirstToken(node);
let rightToken = sourceCode.getLastToken(node);
Expand Down
8 changes: 4 additions & 4 deletions lib/cli-engine.js
Expand Up @@ -542,10 +542,10 @@ class CLIEngine {
if (result.messages.length) {

/*
* if a file contains errors or warnings we don't want to
* store the file in the cache so we can guarantee that
* next execution will also operate on this file
*/
* if a file contains errors or warnings we don't want to
* store the file in the cache so we can guarantee that
* next execution will also operate on this file
*/
fileCache.removeEntry(result.filePath);
} else {

Expand Down
12 changes: 8 additions & 4 deletions lib/code-path-analysis/code-path-analyzer.js
Expand Up @@ -598,8 +598,10 @@ class CodePathAnalyzer {
preprocess(this, node);
}

// Updates the code path.
// And emits onCodePathStart/onCodePathSegmentStart events.
/*
* Updates the code path.
* And emits onCodePathStart/onCodePathSegmentStart events.
*/
processCodePathToEnter(this, node);

// Emits node events.
Expand All @@ -618,8 +620,10 @@ class CodePathAnalyzer {
leaveNode(node) {
this.currentNode = node;

// Updates the code path.
// And emits onCodePathStart/onCodePathSegmentStart events.
/*
* Updates the code path.
* And emits onCodePathStart/onCodePathSegmentStart events.
*/
processCodePathToExit(this, node);

// Emits node events.
Expand Down
6 changes: 4 additions & 2 deletions lib/code-path-analysis/code-path-segment.js
Expand Up @@ -141,8 +141,10 @@ class CodePathSegment {
static newUnreachable(id, allPrevSegments) {
const segment = new CodePathSegment(id, CodePathSegment.flattenUnusedSegments(allPrevSegments), false);

// In `if (a) return a; foo();` case, the unreachable segment preceded by
// the return statement is not used but must not be remove.
/*
* In `if (a) return a; foo();` case, the unreachable segment preceded by
* the return statement is not used but must not be remove.
*/
CodePathSegment.markUsed(segment);

return segment;
Expand Down
6 changes: 4 additions & 2 deletions lib/code-path-analysis/code-path-state.js
Expand Up @@ -774,8 +774,10 @@ class CodePathState {
// Sets the normal path as the next.
this.forkContext.replaceHead(normalSegments);

// If both paths of the `try` block and the `catch` block are
// unreachable, the next path becomes unreachable as well.
/*
* If both paths of the `try` block and the `catch` block are
* unreachable, the next path becomes unreachable as well.
*/
if (!context.lastOfTryIsReachable && !context.lastOfCatchIsReachable) {
this.forkContext.makeUnreachable();
}
Expand Down
20 changes: 12 additions & 8 deletions lib/config.js
Expand Up @@ -120,10 +120,10 @@ class Config {
}

/**
* Loads the config options from a config specified on the command line.
* @param {string} [config] A shareable named config or path to a config file.
* @returns {void}
*/
* Loads the config options from a config specified on the command line.
* @param {string} [config] A shareable named config or path to a config file.
* @returns {void}
*/
loadSpecificConfig(config) {
if (config) {
debug(`Using command line config ${config}`);
Expand Down Expand Up @@ -216,8 +216,10 @@ class Config {
return localConfigHierarchy;
}

// Don't consider the personal config file in the home directory,
// except if the home directory is the same as the current working directory
/*
* Don't consider the personal config file in the home directory,
* except if the home directory is the same as the current working directory
*/
if (localConfigDirectory === PERSONAL_CONFIG_DIR && localConfigFile !== projectConfigPath) {
continue;
}
Expand Down Expand Up @@ -343,8 +345,10 @@ class Config {
this.plugins.loadAll(this.cliConfig.plugins);
}

// Step 3: Override parser only if it is passed explicitly through the command line
// or if it's not defined yet (because the final object will at least have the parser key)
/*
* Step 3: Override parser only if it is passed explicitly through the command line
* or if it's not defined yet (because the final object will at least have the parser key)
*/
if (this.parser || !config.parser) {
config = ConfigOps.merge(config, { parser: this.parser });
}
Expand Down
26 changes: 14 additions & 12 deletions lib/config/autoconfig.js
Expand Up @@ -61,13 +61,13 @@ function makeRegistryItems(rulesConfig) {
}

/**
* Creates an object in which to store rule configs and error counts
*
* Unless a rulesConfig is provided at construction, the registry will not contain
* any rules, only methods. This will be useful for building up registries manually.
*
* Registry class
*/
* Creates an object in which to store rule configs and error counts
*
* Unless a rulesConfig is provided at construction, the registry will not contain
* any rules, only methods. This will be useful for building up registries manually.
*
* Registry class
*/
class Registry {

/**
Expand Down Expand Up @@ -293,11 +293,13 @@ class Registry {

lintResults.forEach(result => {

// It is possible that the error is from a configuration comment
// in a linted file, in which case there may not be a config
// set in this ruleSetIdx.
// (https://github.com/eslint/eslint/issues/5992)
// (https://github.com/eslint/eslint/issues/7860)
/*
* It is possible that the error is from a configuration comment
* in a linted file, in which case there may not be a config
* set in this ruleSetIdx.
* (https://github.com/eslint/eslint/issues/5992)
* (https://github.com/eslint/eslint/issues/7860)
*/
if (
lintedRegistry.rules[result.ruleId] &&
lintedRegistry.rules[result.ruleId][ruleSetIdx]
Expand Down
16 changes: 10 additions & 6 deletions lib/config/config-initializer.js
Expand Up @@ -201,14 +201,18 @@ function configureRules(answers, config) {
// Now that we know which rules to disable, strip out configs with errors
registry = registry.stripFailingConfigs();

// If there is only one config that results in no errors for a rule, we should use it.
// createConfig will only add rules that have one configuration in the registry.
/*
* If there is only one config that results in no errors for a rule, we should use it.
* createConfig will only add rules that have one configuration in the registry.
*/
const singleConfigs = registry.createConfig().rules;

// The "sweet spot" for number of options in a config seems to be two (severity plus one option).
// Very often, a third option (usually an object) is available to address
// edge cases, exceptions, or unique situations. We will prefer to use a config with
// specificity of two.
/*
* The "sweet spot" for number of options in a config seems to be two (severity plus one option).
* Very often, a third option (usually an object) is available to address
* edge cases, exceptions, or unique situations. We will prefer to use a config with
* specificity of two.
*/
const specTwoConfigs = registry.filterBySpecificity(2).createConfig().rules;

// Maybe a specific combination using all three options works
Expand Down

0 comments on commit 8e1a095

Please sign in to comment.