From 5149bb19dfb18e38e0c568de08e9f36d0f4d4426 Mon Sep 17 00:00:00 2001 From: Michael Kauzmann Date: Fri, 22 Mar 2024 08:23:00 -0600 Subject: [PATCH] remove format option to eslint, https://github.com/phetsims/chipper/issues/1427 Signed-off-by: Michael Kauzmann --- eslint/README.md | 12 +-- eslint/format_eslintrc.js | 153 -------------------------------------- js/grunt/Gruntfile.js | 5 -- js/grunt/lint.js | 21 +----- 4 files changed, 6 insertions(+), 185 deletions(-) delete mode 100644 eslint/format_eslintrc.js diff --git a/eslint/README.md b/eslint/README.md index 9192fd0b4..673fb04a2 100644 --- a/eslint/README.md +++ b/eslint/README.md @@ -20,11 +20,8 @@ We use these tools at several stages of development: ## Usage For developers looking to ensure their changes pass ESLint, the typical entry point is to run `grunt lint`. -See `grunt lint --help` for options. If your code passes -`grunt lint`, then it is good to merge. You can add the `--format` option to achieve formatting that comes close to -PhET's code standards defined by the Webstorm code style -[here](https://github.com/phetsims/phet-info/blob/main/ide/idea/phet-idea-codestyle.xml). That is considered the ground -truth for formatting currently, and `--format` attempts to get as close as we can to those rules. +See `grunt lint --help` for options. If your code passes `grunt lint` (and other git hooks), then it is good to +merge. ## Shared Configuration Files @@ -39,15 +36,10 @@ Here is a list of all the available configuration files and why to use them for - `node_eslintrc.js` expands on the base rules and adds configuration only intended for Node.js code (i.e. `perennial`). - `sim_eslintrc.js` expands on the base rules and adds configuration intended for code run in sims (think of this as es5 sim rules) -- `sim_eslintrc.js'` expands on the sim rules and adds configuration for sims that have no es5 in them ( - i.e. `wave-interference`) -- `format_eslintrc.js` contains additional rules used for enforcing code formatting. These are not required, they are - just recommended. So here is the hierarchy of chipper's config files. Indentation symbolized the "extends" relationship. ``` -format_eslintrc.js .eslintrc.js node_eslintrc.js sim_eslintrc.js diff --git a/eslint/format_eslintrc.js b/eslint/format_eslintrc.js deleted file mode 100644 index 59a724627..000000000 --- a/eslint/format_eslintrc.js +++ /dev/null @@ -1,153 +0,0 @@ -// Copyright 2021, University of Colorado Boulder - - -/** - * Additional rules for formatting js. See ./README.md for more info. - * - * @author Sam Reid (PhET Interactive Simulations) - * @author Michael Kauzmann (PhET Interactive Simulations) - */ -module.exports = { - - // SR and MK deleted rules that we think we would never use. - rules: { - 'object-curly-newline': [ 'error', { - ObjectExpression: { minProperties: 4, multiline: true, consistent: true }, - ObjectPattern: { minProperties: 4, multiline: true, consistent: true }, - ImportDeclaration: { minProperties: 4, multiline: true, consistent: true }, - ExportDeclaration: { minProperties: 4, multiline: true, consistent: true } - } ], - - // this option sets a specific tab width for your code - // https://eslint.org/docs/rules/indent - indent: [ 'error', 2, { - SwitchCase: 1, - VariableDeclarator: 'first', - outerIIFEBody: 1, - - // MemberExpression: null, - FunctionDeclaration: { - parameters: 'first', - body: 1 - }, - FunctionExpression: { - parameters: 'first', - body: 1 - }, - CallExpression: { - arguments: 1 - }, - ArrayExpression: 'first', - ObjectExpression: 'first', - ImportDeclaration: 'first', - flatTernaryExpressions: true, - - // list derived from https://github.com/benjamn/ast-types/blob/HEAD/def/jsx.js - ignoredNodes: [ 'ConditionalExpression' ], - ignoreComments: false - } ], - - 'no-multi-spaces': 'error', - - // enforce line breaks after opening and before closing array brackets - // https://eslint.org/docs/rules/array-bracket-newline - // 'array-bracket-newline': [ 'off', 'consistent' ], // object option alternative: { multiline: true, minItems: 3 } - - // enforce line breaks between array elements - // https://eslint.org/docs/rules/array-element-newline - // 'array-element-newline': [ 'off', { multiline: true, minItems: 3 } ], - - // enforce spacing inside single-line blocks - // https://eslint.org/docs/rules/block-spacing - // Leave it off because Webstorm doesn't add spacing to `() => {this.pLDChanged = true;}` to make it `() => { this.pLDChanged = true; } - // perhaps update webstorm rules - // 'block-spacing': [ 'error', 'always' ], - - // enforce consistent line breaks inside function parentheses - // https://eslint.org/docs/rules/function-paren-newline - // I disabled it for compatibility with some formatting I saw in fourier - // 'function-paren-newline': [ 'error', 'consistent' ], - - // Enforce the location of arrow function bodies with implicit returns - // https://eslint.org/docs/rules/implicit-arrow-linebreak - // 'implicit-arrow-linebreak': [ 'error', 'beside' ], - - // specify the maximum length of a line in your program - // https://eslint.org/docs/rules/max-len - // 'max-len': [ 'off', 120, 2, { - // ignoreUrls: true, - // ignoreComments: false, - // ignoreRegExpLiterals: true, - // ignoreStrings: true, - // ignoreTemplateLiterals: true - // } ], - - // limits the number of parameters that can be used in the function declaration. - // 'max-params': [ 'off', 3 ], - - // disallow un-paren'd mixes of different operators - // https://eslint.org/docs/rules/no-mixed-operators - // 'no-mixed-operators': [ 'error', { - // - // // the list of arithmetic groups disallows mixing `%` and `**` - // // with other arithmetic operators. - // groups: [ - // [ '%', '**' ], - // [ '%', '+' ], - // [ '%', '-' ], - // [ '%', '*' ], - // [ '%', '/' ], - // [ '/', '*' ], - // [ '&', '|', '<<', '>>', '>>>' ], - // [ '==', '!=', '===', '!==' ], - // [ '&&', '||' ], - // ], - // allowSamePrecedence: false - // } ], - - // enforce "same line" or "multiple line" on object properties. - // https://eslint.org/docs/rules/object-property-newline - // 'object-property-newline': [ 'error', { - // allowAllPropertiesOnSameLine: true, - // } ], - - // Requires operator at the beginning of the line in multiline statements - // https://eslint.org/docs/rules/operator-linebreak - // 'operator-linebreak': [ 'error', 'after', { overrides: { '=': 'none' } } ], - - // do not require jsdoc - // https://eslint.org/docs/rules/require-jsdoc - // 'require-jsdoc': 'off' - - // require or disallow a space immediately following the // or /* in a comment - // https://eslint.org/docs/rules/spaced-comment - // SR is opinionated about having a space after line comment slashes, but I saw places in fourier where that - // isn't happening, so this is commented out for now. - // 'spaced-comment': [ 'error', 'always', { - // line: { - // exceptions: [ '-', '+' ], - // markers: [ '=', '!', '/' ] // space here to support sprockets directives, slash for TS /// comments - // }, - // block: { - // exceptions: [ '-', '+' ], - // markers: [ '=', '!', ':', '::' ], // space here to support sprockets directives and flow comment types - // balanced: true - // } - // } ], - - /************************************************************** - * Nit Picky rules, we would probably not move these to the primary .eslintrc.js file - *******/ - - // enforce newline at the end of file, with no multiple empty lines - 'eol-last': [ 'error', 'never' ], - - // enforces empty lines around comments - // PhET devs do not want this to be so strict in general - 'lines-around-comment': [ 'error', { beforeLineComment: true } ] - - /******************************** - * End of the Nit Picky rules - *******/ - } -}; \ No newline at end of file diff --git a/js/grunt/Gruntfile.js b/js/grunt/Gruntfile.js index ff9c8c181..59aeaaf4f 100644 --- a/js/grunt/Gruntfile.js +++ b/js/grunt/Gruntfile.js @@ -345,7 +345,6 @@ Minify-specific options: `lint js files. Options: --disable-eslint-cache: cache will not be read or written --fix: autofixable changes will be written to disk ---format: Append an additional set of rules for formatting --chip-away: output a list of responsible devs for each repo with lint problems --disable-with-comment: add an es-lint disable with comment to lint errors --repos: comma separated list of repos to lint in addition to the repo from running`, @@ -355,7 +354,6 @@ Minify-specific options: // --disable-eslint-cache disables the cache, useful for developing rules const cache = !grunt.option( 'disable-eslint-cache' ); const fix = grunt.option( 'fix' ); - const format = grunt.option( 'format' ); const chipAway = grunt.option( 'chip-away' ); const disableWithComment = grunt.option( 'disable-with-comment' ); @@ -364,7 +362,6 @@ Minify-specific options: const lintReturnValue = await lint( [ repo, ...extraRepos ], { cache: cache, fix: fix, - format: format, chipAway: chipAway, disableWithComment: disableWithComment } ); @@ -380,7 +377,6 @@ Minify-specific options: // --disable-eslint-cache disables the cache, useful for developing rules const cache = !grunt.option( 'disable-eslint-cache' ); const fix = grunt.option( 'fix' ); - const format = grunt.option( 'format' ); const chipAway = grunt.option( 'chip-away' ); const disableWithComment = grunt.option( 'disable-with-comment' ); assert && assert( !grunt.option( 'patterns' ), 'patterns not support for lint-all' ); @@ -392,7 +388,6 @@ Minify-specific options: const lintReturnValue = await lint( getPhetLibs( repo, brands ), { cache: cache, fix: fix, - format: format, chipAway: chipAway, disableWithComment: disableWithComment } ); diff --git a/js/grunt/lint.js b/js/grunt/lint.js index 80c975102..71b22b4aa 100644 --- a/js/grunt/lint.js +++ b/js/grunt/lint.js @@ -50,8 +50,7 @@ const lintOneRepo = async ( repo, options ) => { options = _.assignIn( { cache: true, - fix: false, - format: false + fix: false }, options ); // For PhET's custom cache, see CacheLayer @@ -71,7 +70,7 @@ const lintOneRepo = async ( repo, options ) => { const hash = crypto.createHash( 'md5' ).update( tsconfigFile + packageJSON ).digest( 'hex' ); - const eslintConfig = { + const eslint = new ESLint( { // optional auto-fix fix: options.fix, @@ -96,18 +95,7 @@ const lintOneRepo = async ( repo, options ) => { // If no lintable files are found, it is not an error errorOnUnmatchedPattern: false - }; - - const config = {}; - const configExtends = []; - if ( options.format ) { - configExtends.push( '../chipper/eslint/format_eslintrc.js' ); - } - - config.extends = configExtends; - eslintConfig.baseConfig = config; - - const eslint = new ESLint( eslintConfig ); + } ); const results = await eslint.lintFiles( repoToPattern( repo ) ); @@ -148,7 +136,6 @@ const lint = async ( originalRepos, options ) => { options = _.merge( { cache: true, - format: false, // append an extra set of rules for formatting code. TODO: remove this option, these are inside the main rules now, https://github.com/phetsims/chipper/issues/1415 fix: false, // whether fixes should be written to disk chipAway: false, // returns responsible dev info for easier chipping. disableWithComment: false, // replaces failing typescript lines with eslint disable and related comment @@ -198,7 +185,7 @@ const lint = async ( originalRepos, options ) => { } ); worker.postMessage( { repos: batchOfRepos, - options: _.pick( options, [ 'cache', 'format', 'fix' ] ) + options: _.pick( options, [ 'cache', 'fix' ] ) } ); } ); };