From 8610f4c87d426887c07ccfb41d5635e567ed90f9 Mon Sep 17 00:00:00 2001 From: Sam Reid Date: Fri, 6 Sep 2024 15:18:33 -0600 Subject: [PATCH] Add create function to custom eslint rules, see https://github.com/phetsims/chipper/issues/1451 --- eslint/rules/bad-chipper-text.js | 24 +- eslint/rules/bad-phet-library-text.js | 28 +- eslint/rules/bad-sim-text.js | 271 ++++++++--------- eslint/rules/bad-text.js | 286 +++++++++--------- eslint/rules/bad-typescript-text.js | 74 ++--- eslint/rules/copyright.js | 84 ++--- ...-export-class-should-register-namespace.js | 50 +-- eslint/rules/default-export-match-filename.js | 7 +- eslint/rules/default-import-match-filename.js | 62 ++-- eslint/rules/dispose.js | 92 +++--- eslint/rules/namespace-match.js | 44 +-- eslint/rules/no-html-constructors.js | 192 ++++++------ eslint/rules/no-instanceof-array.js | 28 +- .../rules/no-property-in-require-statement.js | 50 +-- .../no-simple-type-checking-assertions.js | 26 +- eslint/rules/no-view-imported-from-model.js | 50 +-- .../phet-io-require-contains-ifphetio.js | 110 +++---- .../rules/property-visibility-annotation.js | 68 +++-- eslint/rules/require-statement-match.js | 106 +++---- eslint/rules/single-line-import.js | 52 ++-- .../uppercase-statics-should-be-readonly.js | 24 +- eslint/rules/visibility-annotation.js | 62 ++-- 22 files changed, 916 insertions(+), 874 deletions(-) diff --git a/eslint/rules/bad-chipper-text.js b/eslint/rules/bad-chipper-text.js index c09108cf..b648aad0 100644 --- a/eslint/rules/bad-chipper-text.js +++ b/eslint/rules/bad-chipper-text.js @@ -1,5 +1,5 @@ // Copyright 2019, University of Colorado Boulder - + /** * Lint detector for invalid text in chipper @@ -9,20 +9,22 @@ * @author Michael Kauzmann (PhET Interactive Simulations) */ -module.exports = function( context ) { +const getBadTextTester = require( './getBadTextTester' ); - const getBadTextTester = require( './getBadTextTester' ); +module.exports = { + create: function( context ) { - // see getBadTextTester for schema. - const forbiddenTextObjects = [ + // see getBadTextTester for schema. + const forbiddenTextObjects = [ - // chipper should use perennial-alias instead of perennial, so that it can check out specific versions - '../perennial/js/' - ]; + // chipper should use perennial-alias instead of perennial, so that it can check out specific versions + '../perennial/js/' + ]; - return { - Program: getBadTextTester( 'bad-chipper-text', forbiddenTextObjects, context ) - }; + return { + Program: getBadTextTester( 'bad-chipper-text', forbiddenTextObjects, context ) + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/bad-phet-library-text.js b/eslint/rules/bad-phet-library-text.js index a0e32178..dec391fc 100644 --- a/eslint/rules/bad-phet-library-text.js +++ b/eslint/rules/bad-phet-library-text.js @@ -11,23 +11,25 @@ * @author Michael Kauzmann (PhET Interactive Simulations) */ -module.exports = function( context ) { +const getBadTextTester = require( './getBadTextTester' ); - const getBadTextTester = require( './getBadTextTester' ); +module.exports = { + create: function( context ) { - // see getBadTextTester for schema. - const forbiddenTextObjects = [ + // see getBadTextTester for schema. + const forbiddenTextObjects = [ - // accessing the sim as a global, like `phet.joist.sim` is a classic example of a hard dependency that can be a - // ticking time bomb for common code that isn't normally run outside phetsims (but could and may want to in the - // future). See https://github.com/phetsims/chipper/issues/1004 - { id: 'phet.joist', codeTokens: [ 'phet', '.', 'joist' ] }, - 'nopedy' - ]; + // accessing the sim as a global, like `phet.joist.sim` is a classic example of a hard dependency that can be a + // ticking time bomb for common code that isn't normally run outside phetsims (but could and may want to in the + // future). See https://github.com/phetsims/chipper/issues/1004 + { id: 'phet.joist', codeTokens: [ 'phet', '.', 'joist' ] }, + 'nopedy' + ]; - return { - Program: getBadTextTester( 'bad-phet-library-text', forbiddenTextObjects, context ) - }; + return { + Program: getBadTextTester( 'bad-phet-library-text', forbiddenTextObjects, context ) + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/bad-sim-text.js b/eslint/rules/bad-sim-text.js index 32b3e98d..5866f6b6 100644 --- a/eslint/rules/bad-sim-text.js +++ b/eslint/rules/bad-sim-text.js @@ -9,143 +9,144 @@ * @author Michael Kauzmann (PhET Interactive Simulations) */ - -module.exports = function( context ) { - - const getBadTextTester = require( './getBadTextTester' ); - - // see getBadTextTester for schema. - const forbiddenTextObjects = [ - - // should be using phet.dot.Utils.roundSymmetric, Math.round does not treat positive and negative numbers - // symmetrically see https://github.com/phetsims/dot/issues/35#issuecomment-113587879 - { id: 'Math.round(', codeTokens: [ 'Math', '.', 'round', '(' ] }, - - // should be using `DOT/dotRandom` - { id: 'Math.random()', codeTokens: [ 'Math', '.', 'random', '(', ')' ] }, - { id: '_.shuffle(', codeTokens: [ '_', '.', 'shuffle', '(' ] }, - { id: '_.sample(', codeTokens: [ '_', '.', 'sample', '(' ] }, - { id: '_.random(', codeTokens: [ '_', '.', 'random', '(' ] }, - { id: 'new Random()', codeTokens: [ 'new', 'Random', '(', ')' ] }, - - // You can use parseInt if you need a non base 10 radix - { id: 'Number.parseInt(', codeTokens: [ 'Number', '.', 'parseInt', '(' ] }, - { id: 'Array.prototype.find', codeTokens: [ 'Array', '.', 'prototype', '.', 'find' ] }, - - // ParallelDOM.pdomOrder should not be mutated, instead only set with `setPDOMOrder` - '.pdomOrder.push(', - - // Should import dotRandom instead of using the namespace - 'phet.dot.dotRandom', - - // Prefer using Pointer.isTouchLike() to help support Pen. This is not set in stone, please see - // https://github.com/phetsims/scenery/issues/1156 and feel free to discuss if there are usages you want to support. - ' instanceof Touch ', - - // Prevent accidental importing of files from the TypeScript build output directory - 'chipper/dist', - - // Relying on these in sim code can break PhET-iO playback, instead use Sim.dimensionProperty, see https://github.com/phetsims/joist/issues/768 - 'window.innerWidth', - 'window.innerHeight', - - // These are types that can be inferred by the common code and provided arguments - 'new Enumeration<', - 'new EnumerationProperty<', - - // Voicing utterances should be registered with a Node for "voicing visibility", using Voicing.alertUtterance asserts - // that. See https://github.com/phetsims/scenery/issues/1403 - 'voicingUtteranceQueue.addToBack', - - // Please use Text/RichText.STRING_PROPERTY_TANDEM_NAME when appropriate (though not all usages apply here, and - // you can ignore this rule), https://github.com/phetsims/scenery/issues/1451#issuecomment-1270576831 - '\'stringProperty\'', - - // Just pass these through, they work with structured cloning as is! See https://github.com/phetsims/tandem/issues/280 - ' NumberIO.toStateObject', - ' NumberIO.fromStateObject', - ' BooleanIO.toStateObject', - ' BooleanIO.fromStateObject', - ' StringIO.toStateObject', - ' StringIO.fromStateObject', - - 'new Tandem(', // use createTandem(), never instantiate your own Tandem please - - // Instead of using your own assertion, see and use Disposable.assertNotDisposable(), https://github.com/phetsims/axon/issues/436 - 'dispose is not supported, exists for the lifetime of the sim', - - // In sims, don't allow setTimout and setInterval calls coming from window, see https://github.com/phetsims/phet-info/issues/59 - { - id: 'setTimeout(', - regex: /(window\.| )setTimeout\(/ - }, - { - id: 'setInterval(', - regex: /(window\.| )setInterval\(/ - }, - - // Decided on during developer meeting, in regards to https://github.com/phetsims/scenery/issues/1324, use `{ Type }` - // import syntax from SCENERY/imports.js - { - id: 'should import from SCENERY/imports.js instead of directly', - regex: /import.*from.*\/scenery\/(?!js\/imports.js)/ - }, - - // Decided on during developer meeting, in regards to https://github.com/phetsims/scenery/issues/1324, use `{ Type }` - // import syntax from KITE/imports.js - { - id: 'should import from KITE/imports.js instead of directly', - regex: /import.*from.*\/kite\/(?!js\/imports.js)/ - }, - - // See https://github.com/phetsims/tandem/issues/302. We don't want to duplicate this type everywhere. Also use 2 - // spaces to prevent false positive for class fields like `public tandem: Tandem;` (which is fine). - { - id: 'Do not duplicate the definition of Tandem, instead use PickRequired', - regex: / {2}tandem\??: Tandem;/ - }, - - // DOT/Utils.toFixed or DOT/Utils.toFixedNumber should be used instead of toFixed. - // JavaScript's toFixed is notoriously buggy. Behavior differs depending on browser, - // because the spec doesn't specify whether to round or floor. - { - id: '.toFixed(', // support regex with english names this way - regex: new RegExp( '(? { - if ( line.trim().indexOf( 'import ' ) === 0 && line.includes( ' from ' ) && ( !line.includes( '.js' ) && !line.includes( '.mjs' ) ) ) { - return false; +const getBadTextTester = require( './getBadTextTester' ); + +module.exports = { + create: function( context ) { + + // see getBadTextTester for schema. + const forbiddenTextObjects = [ + + // should be using phet.dot.Utils.roundSymmetric, Math.round does not treat positive and negative numbers + // symmetrically see https://github.com/phetsims/dot/issues/35#issuecomment-113587879 + { id: 'Math.round(', codeTokens: [ 'Math', '.', 'round', '(' ] }, + + // should be using `DOT/dotRandom` + { id: 'Math.random()', codeTokens: [ 'Math', '.', 'random', '(', ')' ] }, + { id: '_.shuffle(', codeTokens: [ '_', '.', 'shuffle', '(' ] }, + { id: '_.sample(', codeTokens: [ '_', '.', 'sample', '(' ] }, + { id: '_.random(', codeTokens: [ '_', '.', 'random', '(' ] }, + { id: 'new Random()', codeTokens: [ 'new', 'Random', '(', ')' ] }, + + // You can use parseInt if you need a non base 10 radix + { id: 'Number.parseInt(', codeTokens: [ 'Number', '.', 'parseInt', '(' ] }, + { id: 'Array.prototype.find', codeTokens: [ 'Array', '.', 'prototype', '.', 'find' ] }, + + // ParallelDOM.pdomOrder should not be mutated, instead only set with `setPDOMOrder` + '.pdomOrder.push(', + + // Should import dotRandom instead of using the namespace + 'phet.dot.dotRandom', + + // Prefer using Pointer.isTouchLike() to help support Pen. This is not set in stone, please see + // https://github.com/phetsims/scenery/issues/1156 and feel free to discuss if there are usages you want to support. + ' instanceof Touch ', + + // Prevent accidental importing of files from the TypeScript build output directory + 'chipper/dist', + + // Relying on these in sim code can break PhET-iO playback, instead use Sim.dimensionProperty, see https://github.com/phetsims/joist/issues/768 + 'window.innerWidth', + 'window.innerHeight', + + // These are types that can be inferred by the common code and provided arguments + 'new Enumeration<', + 'new EnumerationProperty<', + + // Voicing utterances should be registered with a Node for "voicing visibility", using Voicing.alertUtterance asserts + // that. See https://github.com/phetsims/scenery/issues/1403 + 'voicingUtteranceQueue.addToBack', + + // Please use Text/RichText.STRING_PROPERTY_TANDEM_NAME when appropriate (though not all usages apply here, and + // you can ignore this rule), https://github.com/phetsims/scenery/issues/1451#issuecomment-1270576831 + '\'stringProperty\'', + + // Just pass these through, they work with structured cloning as is! See https://github.com/phetsims/tandem/issues/280 + ' NumberIO.toStateObject', + ' NumberIO.fromStateObject', + ' BooleanIO.toStateObject', + ' BooleanIO.fromStateObject', + ' StringIO.toStateObject', + ' StringIO.fromStateObject', + + 'new Tandem(', // use createTandem(), never instantiate your own Tandem please + + // Instead of using your own assertion, see and use Disposable.assertNotDisposable(), https://github.com/phetsims/axon/issues/436 + 'dispose is not supported, exists for the lifetime of the sim', + + // In sims, don't allow setTimout and setInterval calls coming from window, see https://github.com/phetsims/phet-info/issues/59 + { + id: 'setTimeout(', + regex: /(window\.| )setTimeout\(/ + }, + { + id: 'setInterval(', + regex: /(window\.| )setInterval\(/ + }, + + // Decided on during developer meeting, in regards to https://github.com/phetsims/scenery/issues/1324, use `{ Type }` + // import syntax from SCENERY/imports.js + { + id: 'should import from SCENERY/imports.js instead of directly', + regex: /import.*from.*\/scenery\/(?!js\/imports.js)/ + }, + + // Decided on during developer meeting, in regards to https://github.com/phetsims/scenery/issues/1324, use `{ Type }` + // import syntax from KITE/imports.js + { + id: 'should import from KITE/imports.js instead of directly', + regex: /import.*from.*\/kite\/(?!js\/imports.js)/ + }, + + // See https://github.com/phetsims/tandem/issues/302. We don't want to duplicate this type everywhere. Also use 2 + // spaces to prevent false positive for class fields like `public tandem: Tandem;` (which is fine). + { + id: 'Do not duplicate the definition of Tandem, instead use PickRequired', + regex: / {2}tandem\??: Tandem;/ + }, + + // DOT/Utils.toFixed or DOT/Utils.toFixedNumber should be used instead of toFixed. + // JavaScript's toFixed is notoriously buggy. Behavior differs depending on browser, + // because the spec doesn't specify whether to round or floor. + { + id: '.toFixed(', // support regex with english names this way + regex: new RegExp( '(? { + if ( line.trim().indexOf( 'import ' ) === 0 && line.includes( ' from ' ) && ( !line.includes( '.js' ) && !line.includes( '.mjs' ) ) ) { + return false; + } + return true; } - return true; - } - }, - { - id: 'Import statements require a *.js suffix', - predicate: line => { - if ( line.trim().indexOf( 'import \'' ) === 0 && line.indexOf( ';' ) >= 0 && line.indexOf( '.js' ) === -1 ) { - return false; + }, + { + id: 'Import statements require a *.js suffix', + predicate: line => { + if ( line.trim().indexOf( 'import \'' ) === 0 && line.indexOf( ';' ) >= 0 && line.indexOf( '.js' ) === -1 ) { + return false; + } + return true; } - return true; - } - }, - - { - id: 'Prefer "Standard PhET-iO Wrapper to "standard wrapper"', - regex: /[Ss][Tt][Aa][Nn][Dd][Aa][Rr][Dd][- _][Ww][Rr][Aa][Pp][Pp][Ee][Rr]/ - }, - - // combo box is two words, moved to sim-specific bad text from the general one because of https://github.com/phetsims/website-meteor/issues/690 - 'combobox', // prefer combo box - 'Combobox', // prefer Combo Box - 'COMBOBOX' // prefer COMBO_BOX - ]; - - return { - Program: getBadTextTester( 'bad-sim-text', forbiddenTextObjects, context ) - }; + }, + + { + id: 'Prefer "Standard PhET-iO Wrapper to "standard wrapper"', + regex: /[Ss][Tt][Aa][Nn][Dd][Aa][Rr][Dd][- _][Ww][Rr][Aa][Pp][Pp][Ee][Rr]/ + }, + + // combo box is two words, moved to sim-specific bad text from the general one because of https://github.com/phetsims/website-meteor/issues/690 + 'combobox', // prefer combo box + 'Combobox', // prefer Combo Box + 'COMBOBOX' // prefer COMBO_BOX + ]; + + return { + Program: getBadTextTester( 'bad-sim-text', forbiddenTextObjects, context ) + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/bad-text.js b/eslint/rules/bad-text.js index d2eeb26c..34e38037 100644 --- a/eslint/rules/bad-text.js +++ b/eslint/rules/bad-text.js @@ -9,150 +9,152 @@ */ -module.exports = function( context ) { - - const getBadTextTester = require( './getBadTextTester' ); - - // see getBadTextTester for schema - const forbiddenTextObjects = [ +const getBadTextTester = require( './getBadTextTester' ); + +module.exports = { + create: function( context ) { + + // see getBadTextTester for schema + const forbiddenTextObjects = [ + + // Proper casing for *boxes - // Proper casing for *boxes - - // toolbox is one word - 'toolBox', // prefer toolbox - 'ToolBox', // prefer Toolbox - 'TOOL_BOX', // prefer TOOLBOX - - // checkbox is one word - 'checkBox', // prefer checkbox - 'CheckBox', // prefer Checkbox - 'CHECK_BOX', // prefer CHECKBOX - - 'Overriden', // should have 2 "d"s - 'overriden', // should have 2 "d"s - - 'iFrame', // should be iframe - - // event.keyCode according to spec, rather than event.keycode - 'keycode', - - // prefer hotkey (one word) - 'hot key', - 'hotKey', - 'HotKey', - 'HOT_KEY', - - // embarrassingly enough, zepumph is so bad at typing this word that he needs these lint rules, see https://github.com/phetsims/friction/issues/234 - 'respones', - 'Respones', - - // Avoid string literal in assert predicate, see https://github.com/phetsims/assert/issues/7 - 'assert( \'', - - // In ES6, extending object causes methods to be dropped - { id: 'extends Object ', codeTokens: [ 'extends', 'Object' ] }, - - // Forbid common duplicate words - ' the the ', - ' a a ', - 'dipose', // happens more than you'd think - - // For phet-io use PHET_IO in constants - 'PHETIO', - 'PHET-IO', - 'Phet-iO', - ' Phet ', - 'phetio element', // use "phet-io element" or "PhET-iO Element" - 'PhET-iO element', // use "phet-io element" or "PhET-iO Element" https://github.com/phetsims/phet-io/issues/1968 - 'Phet-iO', - { id: 'IO type', regex: /\bIO type/ }, // https://github.com/phetsims/chipper/issues/977 - // prefer PhET-iO Type name , https://github.com/phetsims/phet-io/issues/1972 - 'IO Typename', - 'IO TypeName', - 'IO Type Name', - - // prefer PhET-iO Type (public name) or IOType (class name) https://github.com/phetsims/phet-io/issues/1972 - 'IO Type', - ' IO type', - - '@return ', - - // see https://thenewstack.io/words-matter-finally-tech-looks-at-removing-exclusionary-language/ and - // https://github.com/phetsims/special-ops/issues/221 - // The regex works around github links that include a `master` branch with a slash, as well as `Ron LeMaster`, a - // PhET team member - { - id: 'words matter', - regex: /(slave|black-?list|white-?list|(? theReturn`, see https://github.com/phetsims/chipper/issues/790 - ' => { return ', - - 'define( function( require ) {', // use define( require => { to standardize before es6 module migration - - // optional 'options' should use brackets and required 'config' shouldn't use brackets, see https://github.com/phetsims/chipper/issues/859 - '@param {Object} options', - '@param {Object} [config]', - - // PhET prefers to use the term "position" to refer to the physical (x,y) position of objects. - // The lint rule can be disabled for occurrences where we do prefer locationProperty, for instance if we - // had a sim about people that are from three different named locations. - 'locationProperty', - - // optionize cannot infer its type arguments, pass them like `optionize()() - 'optionize(', - - // In general, you should not duplicate QueryStringMachine getting phetioDebug, instead just use PhetioClient.prototype.getDebugModeEnabled(), see https://github.com/phetsims/phet-io/issues/1859 - 'phetioDebug:', - - { - id: 'Export statements should not have a register call', - predicate: line => { - if ( line.trim().indexOf( 'export default' ) === 0 && line.indexOf( '.register(' ) >= 0 ) { - return false; + // toolbox is one word + 'toolBox', // prefer toolbox + 'ToolBox', // prefer Toolbox + 'TOOL_BOX', // prefer TOOLBOX + + // checkbox is one word + 'checkBox', // prefer checkbox + 'CheckBox', // prefer Checkbox + 'CHECK_BOX', // prefer CHECKBOX + + 'Overriden', // should have 2 "d"s + 'overriden', // should have 2 "d"s + + 'iFrame', // should be iframe + + // event.keyCode according to spec, rather than event.keycode + 'keycode', + + // prefer hotkey (one word) + 'hot key', + 'hotKey', + 'HotKey', + 'HOT_KEY', + + // embarrassingly enough, zepumph is so bad at typing this word that he needs these lint rules, see https://github.com/phetsims/friction/issues/234 + 'respones', + 'Respones', + + // Avoid string literal in assert predicate, see https://github.com/phetsims/assert/issues/7 + 'assert( \'', + + // In ES6, extending object causes methods to be dropped + { id: 'extends Object ', codeTokens: [ 'extends', 'Object' ] }, + + // Forbid common duplicate words + ' the the ', + ' a a ', + 'dipose', // happens more than you'd think + + // For phet-io use PHET_IO in constants + 'PHETIO', + 'PHET-IO', + 'Phet-iO', + ' Phet ', + 'phetio element', // use "phet-io element" or "PhET-iO Element" + 'PhET-iO element', // use "phet-io element" or "PhET-iO Element" https://github.com/phetsims/phet-io/issues/1968 + 'Phet-iO', + { id: 'IO type', regex: /\bIO type/ }, // https://github.com/phetsims/chipper/issues/977 + // prefer PhET-iO Type name , https://github.com/phetsims/phet-io/issues/1972 + 'IO Typename', + 'IO TypeName', + 'IO Type Name', + + // prefer PhET-iO Type (public name) or IOType (class name) https://github.com/phetsims/phet-io/issues/1972 + 'IO Type', + ' IO type', + + '@return ', + + // see https://thenewstack.io/words-matter-finally-tech-looks-at-removing-exclusionary-language/ and + // https://github.com/phetsims/special-ops/issues/221 + // The regex works around github links that include a `master` branch with a slash, as well as `Ron LeMaster`, a + // PhET team member + { + id: 'words matter', + regex: /(slave|black-?list|white-?list|(? theReturn`, see https://github.com/phetsims/chipper/issues/790 + ' => { return ', + + 'define( function( require ) {', // use define( require => { to standardize before es6 module migration + + // optional 'options' should use brackets and required 'config' shouldn't use brackets, see https://github.com/phetsims/chipper/issues/859 + '@param {Object} options', + '@param {Object} [config]', + + // PhET prefers to use the term "position" to refer to the physical (x,y) position of objects. + // The lint rule can be disabled for occurrences where we do prefer locationProperty, for instance if we + // had a sim about people that are from three different named locations. + 'locationProperty', + + // optionize cannot infer its type arguments, pass them like `optionize()() + 'optionize(', + + // In general, you should not duplicate QueryStringMachine getting phetioDebug, instead just use PhetioClient.prototype.getDebugModeEnabled(), see https://github.com/phetsims/phet-io/issues/1859 + 'phetioDebug:', + + { + id: 'Export statements should not have a register call', + predicate: line => { + if ( line.trim().indexOf( 'export default' ) === 0 && line.indexOf( '.register(' ) >= 0 ) { + return false; + } + return true; } - return true; - } - }, - - // Should have a period before "<", see https://github.com/phetsims/chipper/issues/1005 and https://github.com/phetsims/chipper/issues/1003 - { id: 'Type (add a dot)', regex: /{[^\n:]*[A-z]<[A-z][|'<>A-z]+>[^\n:{}]*}}/ }, - - // eslint disable line directives must have an explanation - { - id: 'eslint-disable-line directives must have explanation', - predicate: line => !line.trim().endsWith( 'eslint-disable-line' ), - - // Report the error on the previous line so it doesn't get disabled - lineNumberDelta: -1 - }, - - // eslint disable next line directives must have an explanation - { - id: 'eslint-disable-next-line directives must have explanation', - predicate: line => !line.trim().endsWith( 'eslint-disable-next-line' ) - }, - - // Prefer _.assignIn() which returns the object in its type doc, https://github.com/phetsims/tasks/issues/1130 - ' = _.extend(' - ]; - - return { - Program: getBadTextTester( 'bad-text', forbiddenTextObjects, context ) - }; + }, + + // Should have a period before "<", see https://github.com/phetsims/chipper/issues/1005 and https://github.com/phetsims/chipper/issues/1003 + { id: 'Type (add a dot)', regex: /{[^\n:]*[A-z]<[A-z][|'<>A-z]+>[^\n:{}]*}}/ }, + + // eslint disable line directives must have an explanation + { + id: 'eslint-disable-line directives must have explanation', + predicate: line => !line.trim().endsWith( 'eslint-disable-line' ), + + // Report the error on the previous line so it doesn't get disabled + lineNumberDelta: -1 + }, + + // eslint disable next line directives must have an explanation + { + id: 'eslint-disable-next-line directives must have explanation', + predicate: line => !line.trim().endsWith( 'eslint-disable-next-line' ) + }, + + // Prefer _.assignIn() which returns the object in its type doc, https://github.com/phetsims/tasks/issues/1130 + ' = _.extend(' + ]; + + return { + Program: getBadTextTester( 'bad-text', forbiddenTextObjects, context ) + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/bad-typescript-text.js b/eslint/rules/bad-typescript-text.js index 0535d61d..b850d059 100644 --- a/eslint/rules/bad-typescript-text.js +++ b/eslint/rules/bad-typescript-text.js @@ -8,56 +8,58 @@ * @author Michael Kauzmann (PhET Interactive Simulations) */ -module.exports = function( context ) { +const getBadTextTester = require( './getBadTextTester' ); - const getBadTextTester = require( './getBadTextTester' ); +module.exports = { + create: function( context ) { - // see getBadTextTester for schema. - const forbiddenTextObjects = [ + // see getBadTextTester for schema. + const forbiddenTextObjects = [ - // Don't lie to yourself. - 'JavaScript is much, much better than TypeScript.', + // Don't lie to yourself. + 'JavaScript is much, much better than TypeScript.', - // Typescript handles this for us, please refrain from providing visibility annotations via jsdoc (unless you have - // to, disabling this rule). - '@public', - '@protected', - '@private', + // Typescript handles this for us, please refrain from providing visibility annotations via jsdoc (unless you have + // to, disabling this rule). + '@public', + '@protected', + '@private', - 'options = merge', + 'options = merge', - // To convert javascript files to typescript, you do not need to include a nocheck directive, just commit locally - // before converting to preserve history, see https://github.com/phetsims/sun/issues/732#issuecomment-995330513 - '@ts-nocheck', + // To convert javascript files to typescript, you do not need to include a nocheck directive, just commit locally + // before converting to preserve history, see https://github.com/phetsims/sun/issues/732#issuecomment-995330513 + '@ts-nocheck', - // combineOptions should always specify the type parameter like combineOptions<...>. - 'combineOptions(', + // combineOptions should always specify the type parameter like combineOptions<...>. + 'combineOptions(', - // The type parameters should be inferred rather than specified - '.multilink<', - '.lazyMultilink<', - 'new DerivedProperty<', + // The type parameters should be inferred rather than specified + '.multilink<', + '.lazyMultilink<', + 'new DerivedProperty<', - 'const simOptions = {', + 'const simOptions = {', - // Typescript files should not use jsdoc for parameters - '@param {', + // Typescript files should not use jsdoc for parameters + '@param {', - // Don't export SelfOptions, https://github.com/phetsims/chipper/issues/1263 - 'export type SelfOptions', + // Don't export SelfOptions, https://github.com/phetsims/chipper/issues/1263 + 'export type SelfOptions', - // Use the PhetioID type alias please, https://github.com/phetsims/tandem/issues/296 - 'phetioID: string', + // Use the PhetioID type alias please, https://github.com/phetsims/tandem/issues/296 + 'phetioID: string', - { - id: '@returns with type and/or without extra doc', - regex: /(@returns \{)|(@returns *$)/ - } - ]; + { + id: '@returns with type and/or without extra doc', + regex: /(@returns \{)|(@returns *$)/ + } + ]; - return { - Program: getBadTextTester( 'bad-typescript-text', forbiddenTextObjects, context ) - }; + return { + Program: getBadTextTester( 'bad-typescript-text', forbiddenTextObjects, context ) + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/copyright.js b/eslint/rules/copyright.js index c629e0e4..6c449ee4 100644 --- a/eslint/rules/copyright.js +++ b/eslint/rules/copyright.js @@ -61,60 +61,62 @@ const needsPhetioLicense = filePath => { _.some( phetioLicenseRepos, repo => localFilePath.startsWith( repo ) ); }; -module.exports = function( context ) { +module.exports = { + create: function( context ) { - return { - Program: function( node ) { - // Get the whole source code, not for node only. - const comments = context.getSourceCode().getAllComments(); + return { + Program: function( node ) { + // Get the whole source code, not for node only. + const comments = context.getSourceCode().getAllComments(); - const filename = context.getFilename(); - const isHTML = filename.endsWith( '.html' ); + const filename = context.getFilename(); + const isHTML = filename.endsWith( '.html' ); - if ( isHTML ) { - return; - } + if ( isHTML ) { + return; + } - const report = ( loc, isPhetio ) => { - context.report( { - node: node, - loc: loc, - message: `Incorrect copyright statement in first comment${isPhetio ? ', phet-io repos require phet-io licensing' : ''}` - } ); - }; + const report = ( loc, isPhetio ) => { + context.report( { + node: node, + loc: loc, + message: `Incorrect copyright statement in first comment${isPhetio ? ', phet-io repos require phet-io licensing' : ''}` + } ); + }; - if ( !comments || comments.length === 0 ) { - report( 1 ); - } - else { - // years must be between 2000 and 2099, inclusive. A script can be used to check that the dates - // match the GitHub creation and last-modified dates - const isDateRangeOK = /^ Copyright 20\d\d-20\d\d, University of Colorado Boulder$/.test( comments[ 0 ].value ); - const isSingleDateOK = /^ Copyright 20\d\d, University of Colorado Boulder$/.test( comments[ 0 ].value ); - if ( !isDateRangeOK && !isSingleDateOK ) { - report( comments[ 0 ].loc.start ); + if ( !comments || comments.length === 0 ) { + report( 1 ); } + else { + // years must be between 2000 and 2099, inclusive. A script can be used to check that the dates + // match the GitHub creation and last-modified dates + const isDateRangeOK = /^ Copyright 20\d\d-20\d\d, University of Colorado Boulder$/.test( comments[ 0 ].value ); + const isSingleDateOK = /^ Copyright 20\d\d, University of Colorado Boulder$/.test( comments[ 0 ].value ); + if ( !isDateRangeOK && !isSingleDateOK ) { + report( comments[ 0 ].loc.start ); + } - else if ( needsPhetioLicense( filename ) ) { - // Handle the case where PhET-iO files need more comments for licensing. + else if ( needsPhetioLicense( filename ) ) { + // Handle the case where PhET-iO files need more comments for licensing. - // phet-io needs 4 line comments at the start of the file. - if ( comments.length < 4 ) { - report( 1, true ); - } + // phet-io needs 4 line comments at the start of the file. + if ( comments.length < 4 ) { + report( 1, true ); + } - const hopefulPhetioComments = comments.slice( 1, 4 ); - for ( let i = 0; i < hopefulPhetioComments.length; i++ ) { - const comment = hopefulPhetioComments[ i ].value; - if ( comment !== phetioComments[ i ] ) { - report( hopefulPhetioComments[ i ].loc.start, true ); - break; + const hopefulPhetioComments = comments.slice( 1, 4 ); + for ( let i = 0; i < hopefulPhetioComments.length; i++ ) { + const comment = hopefulPhetioComments[ i ].value; + if ( comment !== phetioComments[ i ] ) { + report( hopefulPhetioComments[ i ].loc.start, true ); + break; + } } } } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/default-export-class-should-register-namespace.js b/eslint/rules/default-export-class-should-register-namespace.js index 99e52eb1..8fcff187 100644 --- a/eslint/rules/default-export-class-should-register-namespace.js +++ b/eslint/rules/default-export-class-should-register-namespace.js @@ -18,39 +18,41 @@ const _ = require( 'lodash' ); // Repos without a namespace that don't require these register calls. const optOutRepos = [ 'aqua', 'phet-io-wrappers', 'quake', 'studio' ]; -module.exports = function( context ) { +module.exports = { + create: function( context ) { - const filename = context.getFilename(); + const filename = context.getFilename(); - // Javascript string escape the regex escape backslash too (4 backslashes!!) - if ( _.some( optOutRepos, repo => new RegExp( `[\\\\/]${repo}[\\\\/]` ).test( filename ) ) ) { - return {}; // No-op rule - } + // Javascript string escape the regex escape backslash too (4 backslashes!!) + if ( _.some( optOutRepos, repo => new RegExp( `[\\\\/]${repo}[\\\\/]` ).test( filename ) ) ) { + return {}; // No-op rule + } - const classNames = []; - return { + const classNames = []; + return { - ClassDeclaration: node => { - classNames.push( node.id.name ); - }, + ClassDeclaration: node => { + classNames.push( node.id.name ); + }, - ExportDefaultDeclaration: node => { + ExportDefaultDeclaration: node => { - // Has a class default export - if ( node.declaration.type === 'ClassDeclaration' || - ( node.declaration && node.declaration.name && classNames.includes( node.declaration.name ) ) ) { + // Has a class default export + if ( node.declaration.type === 'ClassDeclaration' || + ( node.declaration && node.declaration.name && classNames.includes( node.declaration.name ) ) ) { - // No register call - if ( !context.getSourceCode().text.includes( '.register( \'' ) ) { - context.report( { - node: node, - loc: node.loc, - message: 'File default exports a class but has no namespace registration' - } ); + // No register call + if ( !context.getSourceCode().text.includes( '.register( \'' ) ) { + context.report( { + node: node, + loc: node.loc, + message: 'File default exports a class but has no namespace registration' + } ); + } } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/default-export-match-filename.js b/eslint/rules/default-export-match-filename.js index 6d57f9e0..b3615a60 100644 --- a/eslint/rules/default-export-match-filename.js +++ b/eslint/rules/default-export-match-filename.js @@ -7,8 +7,8 @@ * @author Sam Reid (PhET Interactive Simulations) */ -module.exports = - context => { +module.exports = { + create: context => { return { ExportDefaultDeclaration: node => { @@ -29,4 +29,5 @@ module.exports = } } }; - }; \ No newline at end of file + } +}; \ No newline at end of file diff --git a/eslint/rules/default-import-match-filename.js b/eslint/rules/default-import-match-filename.js index 527d0483..474636a8 100644 --- a/eslint/rules/default-import-match-filename.js +++ b/eslint/rules/default-import-match-filename.js @@ -106,35 +106,37 @@ function getFilename( filePath, context ) { return path.basename( processedPath ) + ( isDir ? '/' : '' ); } -module.exports = function( context ) { - - return { - ImportDeclaration( node ) { - const defaultImportSpecifier = node.specifiers.find( - ( { type } ) => type === 'ImportDefaultSpecifier' - ); - - const defaultImportName = - defaultImportSpecifier && defaultImportSpecifier.local.name; - - if ( !defaultImportName ) { - return; +module.exports = { + create: function( context ) { + + return { + ImportDeclaration( node ) { + const defaultImportSpecifier = node.specifiers.find( + ( { type } ) => type === 'ImportDefaultSpecifier' + ); + + const defaultImportName = + defaultImportSpecifier && defaultImportSpecifier.local.name; + + if ( !defaultImportName ) { + return; + } + + const filename = getFilename( node.source.value, context ); + + if ( !filename ) { + return; + } + + if ( + !isCompatible( defaultImportName, filename ) + ) { + context.report( { + node: defaultImportSpecifier, + message: `Default import name does not match filename "${filename}".` + } ); + } } - - const filename = getFilename( node.source.value, context ); - - if ( !filename ) { - return; - } - - if ( - !isCompatible( defaultImportName, filename ) - ) { - context.report( { - node: defaultImportSpecifier, - message: `Default import name does not match filename "${filename}".` - } ); - } - } - }; + }; + } }; \ No newline at end of file diff --git a/eslint/rules/dispose.js b/eslint/rules/dispose.js index 3ccf1427..60c7cf26 100644 --- a/eslint/rules/dispose.js +++ b/eslint/rules/dispose.js @@ -10,61 +10,63 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { +module.exports = { + create: function( context ) { - // the following holds the possible ways to register various PhET listeners and observers - // TODO: derivedProperty, https://github.com/phetsims/chipper/issues/418 - const OBSERVER_REGISTRATIONS = { - LINK: 'link', - LAZY_LINK: 'lazyLink', - ON: 'on', - MULTILINK: 'multilink', - ADD_LISTENER: 'addListener', - ADD_EVENT_LISTENER: 'addEventListener', - ADD_INSTANCE: 'addInstance' - }; + // the following holds the possible ways to register various PhET listeners and observers + // TODO: derivedProperty, https://github.com/phetsims/chipper/issues/418 + const OBSERVER_REGISTRATIONS = { + LINK: 'link', + LAZY_LINK: 'lazyLink', + ON: 'on', + MULTILINK: 'multilink', + ADD_LISTENER: 'addListener', + ADD_EVENT_LISTENER: 'addEventListener', + ADD_INSTANCE: 'addInstance' + }; - return { + return { - ExpressionStatement: function( node ) { + ExpressionStatement: function( node ) { - // look through the AST of a typical observer registration, see https://github.com/phetsims/chipper/issues/418 - if ( node.expression && - node.expression.callee && - node.expression.callee.property && - node.expression.callee.property.name ) { - const calleeName = node.expression.callee.property.name; - for ( const key in OBSERVER_REGISTRATIONS ) { - if ( OBSERVER_REGISTRATIONS.hasOwnProperty( key ) ) { - if ( calleeName === OBSERVER_REGISTRATIONS[ key ] ) { - // we have found an observer registration, start at the root and look through its tokens for dispose - let disposeFound = false; - const rootNode = context.getSourceCode().ast; - if ( rootNode && - rootNode.tokens ) { - rootNode.tokens.forEach( token => { - if ( token ) { - if ( token.type === 'Identifier' && - token.value === 'dispose' ) { - // we have found a dispose function - disposeFound = true; + // look through the AST of a typical observer registration, see https://github.com/phetsims/chipper/issues/418 + if ( node.expression && + node.expression.callee && + node.expression.callee.property && + node.expression.callee.property.name ) { + const calleeName = node.expression.callee.property.name; + for ( const key in OBSERVER_REGISTRATIONS ) { + if ( OBSERVER_REGISTRATIONS.hasOwnProperty( key ) ) { + if ( calleeName === OBSERVER_REGISTRATIONS[ key ] ) { + // we have found an observer registration, start at the root and look through its tokens for dispose + let disposeFound = false; + const rootNode = context.getSourceCode().ast; + if ( rootNode && + rootNode.tokens ) { + rootNode.tokens.forEach( token => { + if ( token ) { + if ( token.type === 'Identifier' && + token.value === 'dispose' ) { + // we have found a dispose function + disposeFound = true; + } } - } - } ); - } - if ( !disposeFound ) { - context.report( { - node: node, - loc: node.loc.start, - message: 'observer registration missing dispose function' - } ); + } ); + } + if ( !disposeFound ) { + context.report( { + node: node, + loc: node.loc.start, + message: 'observer registration missing dispose function' + } ); + } } } } } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/namespace-match.js b/eslint/rules/namespace-match.js index 9482426a..9c6890bc 100644 --- a/eslint/rules/namespace-match.js +++ b/eslint/rules/namespace-match.js @@ -45,34 +45,36 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { - return { - CallExpression: node => { - if ( node.callee && node.callee.type === 'MemberExpression' ) { - if ( node.callee.property.name === 'register' ) { - const registerArgs = node.arguments; - if ( registerArgs.length === 2 ) { - const firstArg = registerArgs[ 0 ]; - const secondArg = registerArgs[ 1 ]; +module.exports = { + create: function( context ) { + return { + CallExpression: node => { + if ( node.callee && node.callee.type === 'MemberExpression' ) { + if ( node.callee.property.name === 'register' ) { + const registerArgs = node.arguments; + if ( registerArgs.length === 2 ) { + const firstArg = registerArgs[ 0 ]; + const secondArg = registerArgs[ 1 ]; - // we allow adding functions directly to the namespace - if ( firstArg.type === 'Literal' && secondArg.type === 'Identifier' ) { - const firstArgValue = firstArg.value; // string from the Literal node value - const secondArgName = secondArg.name; // string from the Identifier node name + // we allow adding functions directly to the namespace + if ( firstArg.type === 'Literal' && secondArg.type === 'Identifier' ) { + const firstArgValue = firstArg.value; // string from the Literal node value + const secondArgName = secondArg.name; // string from the Identifier node name - if ( firstArgValue !== secondArgName ) { - context.report( { - node: node, - loc: node.loc, - message: `namespace key must match value - key: ${firstArgValue}, value: ${secondArgName}` - } ); + if ( firstArgValue !== secondArgName ) { + context.report( { + node: node, + loc: node.loc, + message: `namespace key must match value - key: ${firstArgValue}, value: ${secondArgName}` + } ); + } } } } } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/no-html-constructors.js b/eslint/rules/no-html-constructors.js index 364c49cf..1deefb3f 100644 --- a/eslint/rules/no-html-constructors.js +++ b/eslint/rules/no-html-constructors.js @@ -22,120 +22,122 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { +module.exports = { + create: function( context ) { - // names of the native JavaScript constructors that clash with PhET type names - const nativeConstructors = [ 'Image', 'Range', 'Text', 'Node', 'Event' ]; + // names of the native JavaScript constructors that clash with PhET type names + const nativeConstructors = [ 'Image', 'Range', 'Text', 'Node', 'Event' ]; - // list of all types that are declared in the file that have the same name as a native JavaScript constructor - const declaredTypes = []; - - /** - * Add a type to declared types if the 'declaration' node has a name which is equal to - * one of the entries in nativeConstructors. Called when eslint finds VariableDeclarator, FunctionDeclaration - * or ImportDeclaration nodes as it traverses down the AST. - * - * @param {ASTNode} node - */ - function addDeclaredType( node ) { - - // Example... - // - // JavaScript: - // - // function Range( min, max ) {...} - // - // Corresponding AST: - // - // FunctionDeclaration { - // type: "FunctionDeclaration", - // id: Identifier { - // type: "Identifier", - // name: "Range" - // } - // } - - if ( node.id && nativeConstructors.indexOf( node.id.name ) !== -1 ) { - declaredTypes.push( node.id.name ); - } - - // Example... - // - // JavaScript: - // - // import Range from '../../dot/js/Range.js' - // - // Corresponding AST: - // - // { - // "type": "ImportDeclaration", - // "specifiers": [ - // { - // "type": "ImportDefaultSpecifier", - // "local": { - // "type": "Identifier", - // "name": "Range" - // }, - // } - // ] - // } - if ( node.specifiers ) { - node.specifiers.forEach( specifier => { - if ( specifier.local && specifier.local.name ) { - if ( nativeConstructors.indexOf( specifier.local.name ) !== -1 ) { - declaredTypes.push( specifier.local.name ); - } - } - } ); - } - } - - return { - VariableDeclarator: addDeclaredType, - FunctionDeclaration: addDeclaredType, - ImportDeclaration: addDeclaredType, + // list of all types that are declared in the file that have the same name as a native JavaScript constructor + const declaredTypes = []; /** - * When eslint traverses back up the AST, search for a node representing an instantiation of a type. If found, - * check to see if the type being instantiated is one of the entries that were added to declaredTypes on - * the first traversal down the AST. + * Add a type to declared types if the 'declaration' node has a name which is equal to + * one of the entries in nativeConstructors. Called when eslint finds VariableDeclarator, FunctionDeclaration + * or ImportDeclaration nodes as it traverses down the AST. * - * @param {ASTNode} node + * @param {ASTNode} node */ - 'NewExpression:exit': function( node ) { + function addDeclaredType( node ) { // Example... // // JavaScript: // - // var imageNode = new Image( imgsrc ); + // function Range( min, max ) {...} // // Corresponding AST: // - // exemplar: { - // { - // "type": "NewExpression", - // "callee": { - // "type": "Identifier", - // "name": "Image" - // } + // FunctionDeclaration { + // type: "FunctionDeclaration", + // id: Identifier { + // type: "Identifier", + // name: "Range" // } // } - if ( node.callee && node.callee.name && nativeConstructors.indexOf( node.callee.name ) !== -1 ) { - const constructorName = node.callee.name; - const filename = context.getFilename(); - const constructorUsedInOwnFile = filename.endsWith( `${constructorName}.ts` ); - if ( !constructorUsedInOwnFile && declaredTypes.indexOf( constructorName ) === -1 ) { - context.report( { - node: node, - loc: node.callee.loc, - message: `${constructorName}: using native constructor instead of project module, did you forget an import statement?` - } ); - } + if ( node.id && nativeConstructors.indexOf( node.id.name ) !== -1 ) { + declaredTypes.push( node.id.name ); + } + + // Example... + // + // JavaScript: + // + // import Range from '../../dot/js/Range.js' + // + // Corresponding AST: + // + // { + // "type": "ImportDeclaration", + // "specifiers": [ + // { + // "type": "ImportDefaultSpecifier", + // "local": { + // "type": "Identifier", + // "name": "Range" + // }, + // } + // ] + // } + if ( node.specifiers ) { + node.specifiers.forEach( specifier => { + if ( specifier.local && specifier.local.name ) { + if ( nativeConstructors.indexOf( specifier.local.name ) !== -1 ) { + declaredTypes.push( specifier.local.name ); + } + } + } ); } } - }; + + return { + VariableDeclarator: addDeclaredType, + FunctionDeclaration: addDeclaredType, + ImportDeclaration: addDeclaredType, + + /** + * When eslint traverses back up the AST, search for a node representing an instantiation of a type. If found, + * check to see if the type being instantiated is one of the entries that were added to declaredTypes on + * the first traversal down the AST. + * + * @param {ASTNode} node + */ + 'NewExpression:exit': function( node ) { + + // Example... + // + // JavaScript: + // + // var imageNode = new Image( imgsrc ); + // + // Corresponding AST: + // + // exemplar: { + // { + // "type": "NewExpression", + // "callee": { + // "type": "Identifier", + // "name": "Image" + // } + // } + // } + + if ( node.callee && node.callee.name && nativeConstructors.indexOf( node.callee.name ) !== -1 ) { + const constructorName = node.callee.name; + const filename = context.getFilename(); + const constructorUsedInOwnFile = filename.endsWith( `${constructorName}.ts` ); + if ( !constructorUsedInOwnFile && declaredTypes.indexOf( constructorName ) === -1 ) { + context.report( { + node: node, + loc: node.callee.loc, + message: `${constructorName}: using native constructor instead of project module, did you forget an import statement?` + } ); + } + } + } + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/no-instanceof-array.js b/eslint/rules/no-instanceof-array.js index 7a5c45b9..d1a9e8cd 100644 --- a/eslint/rules/no-instanceof-array.js +++ b/eslint/rules/no-instanceof-array.js @@ -10,21 +10,23 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { +module.exports = { + create: function( context ) { - return { - BinaryExpression: function( node ) { - if ( node.operator === 'instanceof' && node.right.type === 'Identifier' && node.right.name === 'Array' ) { - context.report( { - node: node, - message: 'Use Array.isArray() instead of instanceof Array', - data: { - identifier: node.name - } - } ); + return { + BinaryExpression: function( node ) { + if ( node.operator === 'instanceof' && node.right.type === 'Identifier' && node.right.name === 'Array' ) { + context.report( { + node: node, + message: 'Use Array.isArray() instead of instanceof Array', + data: { + identifier: node.name + } + } ); + } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/no-property-in-require-statement.js b/eslint/rules/no-property-in-require-statement.js index 2b0c5222..9e0cc101 100644 --- a/eslint/rules/no-property-in-require-statement.js +++ b/eslint/rules/no-property-in-require-statement.js @@ -10,31 +10,33 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { - - return { - - VariableDeclaration: function( node ) { - - if ( node.declarations && - node.declarations.length > 0 && - node.declarations[ 0 ] && - node.declarations[ 0 ].init && - node.declarations[ 0 ].init.type && - node.declarations[ 0 ].init.type === 'MemberExpression' && - node.declarations[ 0 ].init.object && - node.declarations[ 0 ].init.object.callee && - node.declarations[ 0 ].init.object.callee.name && - node.declarations[ 0 ].init.object.callee.name === 'require' ) { - - context.report( { - node: node, - loc: node.loc.start, - message: 'property access in require statement' - } ); +module.exports = { + create: function( context ) { + + return { + + VariableDeclaration: function( node ) { + + if ( node.declarations && + node.declarations.length > 0 && + node.declarations[ 0 ] && + node.declarations[ 0 ].init && + node.declarations[ 0 ].init.type && + node.declarations[ 0 ].init.type === 'MemberExpression' && + node.declarations[ 0 ].init.object && + node.declarations[ 0 ].init.object.callee && + node.declarations[ 0 ].init.object.callee.name && + node.declarations[ 0 ].init.object.callee.name === 'require' ) { + + context.report( { + node: node, + loc: node.loc.start, + message: 'property access in require statement' + } ); + } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/no-simple-type-checking-assertions.js b/eslint/rules/no-simple-type-checking-assertions.js index 2cbcc35d..675d0770 100644 --- a/eslint/rules/no-simple-type-checking-assertions.js +++ b/eslint/rules/no-simple-type-checking-assertions.js @@ -13,21 +13,23 @@ * @author Michael Kauzmann (PhET Interactive Simulations) */ -module.exports = function( context ) { +const getBadTextTester = require( './getBadTextTester' ); - const getBadTextTester = require( './getBadTextTester' ); +module.exports = { + create: function( context ) { - // see getBadTextTester for schema. - const forbiddenTextObject = [ - { - id: 'asserting values are instanceof or typeof in TypeScript is redundant', - regex: /assert && assert\( ((\w+ instanceof \w+)|(typeof \w+ === '\w+'))(,| \))/ - } - ]; + // see getBadTextTester for schema. + const forbiddenTextObject = [ + { + id: 'asserting values are instanceof or typeof in TypeScript is redundant', + regex: /assert && assert\( ((\w+ instanceof \w+)|(typeof \w+ === '\w+'))(,| \))/ + } + ]; - return { - Program: getBadTextTester( 'bad-typescript-text', forbiddenTextObject, context ) - }; + return { + Program: getBadTextTester( 'bad-typescript-text', forbiddenTextObject, context ) + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/no-view-imported-from-model.js b/eslint/rules/no-view-imported-from-model.js index 5411a392..92367f2b 100644 --- a/eslint/rules/no-view-imported-from-model.js +++ b/eslint/rules/no-view-imported-from-model.js @@ -15,33 +15,35 @@ const isModelFileRegex = /[\\/]model[\\/]/; const isViewFileRegex = /[\\/]view[\\/]/; const isModelScreenViewFolder = /[\\/]model[\\/]view[\\/]/; // for the rare case where a `model` screen has a `view` folder. -module.exports = context => { - const filename = context.getFilename(); - - // select paths like "/model/" without the false positive of "/model/view/" which could happen if the screen was model - if ( isModelFileRegex.test( filename ) && !isModelScreenViewFolder.test( filename ) ) { - return { - ImportDeclaration: node => { - const importValue = node.source.value; - - // If the import has /view/ in it. - if ( isViewFileRegex.test( importValue ) ) { - - // Some special cases that are too common for PhET to care about this failure for. - if ( node.importKind !== 'type' && // importing is not as bad - !importValue.endsWith( 'Colors.js' ) && // Colors files are auto generated and in the view - !importValue.endsWith( 'ModelViewTransform2.js' ) ) { // Enough cases to warrant taking it out here. - context.report( { - node: node, - loc: node.loc, - message: `model import statement should not import the view: ${importValue.replace( '/..', '' )}` - } ); +module.exports = { + create: context => { + const filename = context.getFilename(); + + // select paths like "/model/" without the false positive of "/model/view/" which could happen if the screen was model + if ( isModelFileRegex.test( filename ) && !isModelScreenViewFolder.test( filename ) ) { + return { + ImportDeclaration: node => { + const importValue = node.source.value; + + // If the import has /view/ in it. + if ( isViewFileRegex.test( importValue ) ) { + + // Some special cases that are too common for PhET to care about this failure for. + if ( node.importKind !== 'type' && // importing is not as bad + !importValue.endsWith( 'Colors.js' ) && // Colors files are auto generated and in the view + !importValue.endsWith( 'ModelViewTransform2.js' ) ) { // Enough cases to warrant taking it out here. + context.report( { + node: node, + loc: node.loc, + message: `model import statement should not import the view: ${importValue.replace( '/..', '' )}` + } ); + } } } - } - }; + }; + } + return {}; } - return {}; }; module.exports.schema = [ diff --git a/eslint/rules/phet-io-require-contains-ifphetio.js b/eslint/rules/phet-io-require-contains-ifphetio.js index b8ac57fa..31bc9b9f 100644 --- a/eslint/rules/phet-io-require-contains-ifphetio.js +++ b/eslint/rules/phet-io-require-contains-ifphetio.js @@ -10,72 +10,74 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { +module.exports = { + create: function( context ) { - return { + return { - VariableDeclaration: function( node ) { + VariableDeclaration: function( node ) { - // Here is the AST of a typical require statement node, for reference - // ( should be the same comment as in require-statement-match.js) - // - //var exemplar = { - // 'type': 'VariableDeclaration', - // 'declarations': [ - // { - // 'type': 'VariableDeclarator', - // 'id': { - // 'type': 'Identifier', - // 'name': 'EquationsScreen' - // }, - // 'init': { - // 'type': 'CallExpression', - // 'callee': { - // 'type': 'Identifier', - // 'name': 'require' - // }, - // 'arguments': [ - // { - // 'type': 'Literal', - // 'value': 'FUNCTION_BUILDER/equations/EquationsScreen', - // 'raw': "'FUNCTION_BUILDER/equations/EquationsScreen'"// eslint-disable-line ??? - // } - // ] - // } - // } - // ], - // 'kind': 'var' - //}; + // Here is the AST of a typical require statement node, for reference + // ( should be the same comment as in require-statement-match.js) + // + //var exemplar = { + // 'type': 'VariableDeclaration', + // 'declarations': [ + // { + // 'type': 'VariableDeclarator', + // 'id': { + // 'type': 'Identifier', + // 'name': 'EquationsScreen' + // }, + // 'init': { + // 'type': 'CallExpression', + // 'callee': { + // 'type': 'Identifier', + // 'name': 'require' + // }, + // 'arguments': [ + // { + // 'type': 'Literal', + // 'value': 'FUNCTION_BUILDER/equations/EquationsScreen', + // 'raw': "'FUNCTION_BUILDER/equations/EquationsScreen'"// eslint-disable-line ??? + // } + // ] + // } + // } + // ], + // 'kind': 'var' + //}; - if ( node.declarations && - node.declarations.length > 0 && - node.declarations[ 0 ].init && - node.declarations[ 0 ].init.arguments && - node.declarations[ 0 ].init.arguments.length > 0 && - node.declarations[ 0 ].init.callee.name === 'require' ) { + if ( node.declarations && + node.declarations.length > 0 && + node.declarations[ 0 ].init && + node.declarations[ 0 ].init.arguments && + node.declarations[ 0 ].init.arguments.length > 0 && + node.declarations[ 0 ].init.callee.name === 'require' ) { - const rhs = node.declarations[ 0 ].init.arguments[ 0 ].value; + const rhs = node.declarations[ 0 ].init.arguments[ 0 ].value; - // If there is a PHET_IO import, but ifphetio! isn't the first part of the require - if ( rhs && rhs.indexOf( 'PHET_IO/' ) >= 0 && rhs.indexOf( 'ifphetio!' ) !== 0 ) { + // If there is a PHET_IO import, but ifphetio! isn't the first part of the require + if ( rhs && rhs.indexOf( 'PHET_IO/' ) >= 0 && rhs.indexOf( 'ifphetio!' ) !== 0 ) { - // This regex will match 'phet-io' plus either a '/' or a '\' afterwards. - const regex = /phet-io[/\\]/; + // This regex will match 'phet-io' plus either a '/' or a '\' afterwards. + const regex = /phet-io[/\\]/; - // Don't check this rule in the phet-io repo - // Match returns null if it doesn't find anything - if ( !context.getFilename().match( regex ) ) { - context.report( { - node: node, - loc: node.loc.start, - message: 'PHET_IO require must start with an ifphetio! check. ' - } ); + // Don't check this rule in the phet-io repo + // Match returns null if it doesn't find anything + if ( !context.getFilename().match( regex ) ) { + context.report( { + node: node, + loc: node.loc.start, + message: 'PHET_IO require must start with an ifphetio! check. ' + } ); + } } } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/property-visibility-annotation.js b/eslint/rules/property-visibility-annotation.js index 2782d280..16f8fd2a 100644 --- a/eslint/rules/property-visibility-annotation.js +++ b/eslint/rules/property-visibility-annotation.js @@ -11,53 +11,55 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { +module.exports = { + create: function( context ) { - return { + return { - AssignmentExpression: function( node ) { - let isAnnotated = false; + AssignmentExpression: function( node ) { + let isAnnotated = false; - if ( node.left && node.left && node.left.object && node.left.object.type === 'ThisExpression' ) { - const leadingComments = node.parent.leadingComments; - let i; - let a; - if ( leadingComments ) { - for ( i = 0; i < leadingComments.length; i++ ) { - a = leadingComments[ i ]; - if ( a.value.indexOf( '@public' ) >= 0 || a.value.indexOf( '@private' ) >= 0 ) { - isAnnotated = true; + if ( node.left && node.left && node.left.object && node.left.object.type === 'ThisExpression' ) { + const leadingComments = node.parent.leadingComments; + let i; + let a; + if ( leadingComments ) { + for ( i = 0; i < leadingComments.length; i++ ) { + a = leadingComments[ i ]; + if ( a.value.indexOf( '@public' ) >= 0 || a.value.indexOf( '@private' ) >= 0 ) { + isAnnotated = true; + } } } - } - const trailingComments = node.parent.trailingComments; - if ( trailingComments ) { - for ( i = 0; i < trailingComments.length; i++ ) { - a = trailingComments[ i ]; - if ( a.value.indexOf( '@public' ) >= 0 || a.value.indexOf( '@private' ) >= 0 ) { - isAnnotated = true; + const trailingComments = node.parent.trailingComments; + if ( trailingComments ) { + for ( i = 0; i < trailingComments.length; i++ ) { + a = trailingComments[ i ]; + if ( a.value.indexOf( '@public' ) >= 0 || a.value.indexOf( '@private' ) >= 0 ) { + isAnnotated = true; + } } } } - } - if ( node.parent && node.parent.parent && node.parent.parent.parent ) { - const parentFunction = node.parent.parent.parent; - if ( parentFunction.id && parentFunction.id.name ) { - if ( parentFunction.type === 'FunctionDeclaration' && parentFunction.id.name[ 0 ].toUpperCase() === parentFunction.id.name[ 0 ] ) { - if ( !isAnnotated ) { - context.report( { - node: node, - loc: node.loc.start, - message: 'missing visibility annotation' - } ); + if ( node.parent && node.parent.parent && node.parent.parent.parent ) { + const parentFunction = node.parent.parent.parent; + if ( parentFunction.id && parentFunction.id.name ) { + if ( parentFunction.type === 'FunctionDeclaration' && parentFunction.id.name[ 0 ].toUpperCase() === parentFunction.id.name[ 0 ] ) { + if ( !isAnnotated ) { + context.report( { + node: node, + loc: node.loc.start, + message: 'missing visibility annotation' + } ); + } } } } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/require-statement-match.js b/eslint/rules/require-statement-match.js index 259d6a01..0f08fa1d 100644 --- a/eslint/rules/require-statement-match.js +++ b/eslint/rules/require-statement-match.js @@ -10,68 +10,70 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { +module.exports = { + create: function( context ) { - return { + return { - VariableDeclaration: function( node ) { + VariableDeclaration: function( node ) { - // Here is the AST of a typical require statement node, for reference - //var exemplar = { - // 'type': 'VariableDeclaration', - // 'declarations': [ - // { - // 'type': 'VariableDeclarator', - // 'id': { - // 'type': 'Identifier', - // 'name': 'EquationsScreen' - // }, - // 'init': { - // 'type': 'CallExpression', - // 'callee': { - // 'type': 'Identifier', - // 'name': 'require' - // }, - // 'arguments': [ - // { - // 'type': 'Literal', - // 'value': 'FUNCTION_BUILDER/equations/EquationsScreen', - // 'raw': "'FUNCTION_BUILDER/equations/EquationsScreen'" - // } - // ] - // } - // } - // ], - // 'kind': 'var' - //}; + // Here is the AST of a typical require statement node, for reference + //var exemplar = { + // 'type': 'VariableDeclaration', + // 'declarations': [ + // { + // 'type': 'VariableDeclarator', + // 'id': { + // 'type': 'Identifier', + // 'name': 'EquationsScreen' + // }, + // 'init': { + // 'type': 'CallExpression', + // 'callee': { + // 'type': 'Identifier', + // 'name': 'require' + // }, + // 'arguments': [ + // { + // 'type': 'Literal', + // 'value': 'FUNCTION_BUILDER/equations/EquationsScreen', + // 'raw': "'FUNCTION_BUILDER/equations/EquationsScreen'" + // } + // ] + // } + // } + // ], + // 'kind': 'var' + //}; - if ( node.declarations && - node.declarations.length > 0 && - node.declarations[ 0 ].init && - node.declarations[ 0 ].init.arguments && - node.declarations[ 0 ].init.arguments.length > 0 ) { - if ( node.declarations[ 0 ].init && - node.declarations[ 0 ].init.callee.name === 'require' ) { - const lhs = node.declarations[ 0 ].id.name; - const rhs = node.declarations[ 0 ].init.arguments[ 0 ].value; + if ( node.declarations && + node.declarations.length > 0 && + node.declarations[ 0 ].init && + node.declarations[ 0 ].init.arguments && + node.declarations[ 0 ].init.arguments.length > 0 ) { + if ( node.declarations[ 0 ].init && + node.declarations[ 0 ].init.callee.name === 'require' ) { + const lhs = node.declarations[ 0 ].id.name; + const rhs = node.declarations[ 0 ].init.arguments[ 0 ].value; - if ( rhs && rhs.indexOf( '!' ) < 0 ) { - const lastSlash = rhs.lastIndexOf( '/' ); - const tail = rhs.substring( lastSlash + 1 ); + if ( rhs && rhs.indexOf( '!' ) < 0 ) { + const lastSlash = rhs.lastIndexOf( '/' ); + const tail = rhs.substring( lastSlash + 1 ); - const isLodash = lhs === '_'; - if ( tail !== lhs && !isLodash ) { - context.report( { - node: node, - loc: node.loc.start, - message: `Mismatched require statement values, ${lhs} !== ${tail}` - } ); + const isLodash = lhs === '_'; + if ( tail !== lhs && !isLodash ) { + context.report( { + node: node, + loc: node.loc.start, + message: `Mismatched require statement values, ${lhs} !== ${tail}` + } ); + } } } } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/single-line-import.js b/eslint/rules/single-line-import.js index ef9fd87f..5bdd1f3f 100644 --- a/eslint/rules/single-line-import.js +++ b/eslint/rules/single-line-import.js @@ -12,36 +12,38 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = context => { +module.exports = { + create: context => { - return { + return { - ImportDeclaration: node => { - if ( node.loc.start.line !== node.loc.end.line ) { + ImportDeclaration: node => { + if ( node.loc.start.line !== node.loc.end.line ) { - // AST JSON might look something like: - // { - // "type": "ImportDeclaration", - // "specifiers": [ - // { - // "type": "ImportDefaultSpecifier", - // "local": { - // "type": "Identifier", - // "name": "EnergySkateParkColorScheme", - // } - // } - // ] - // } - node.specifiers.forEach( specifier => { - context.report( { - node: node, - loc: node.loc, - message: `${specifier.local.name}: import statement should be on a single line.` + // AST JSON might look something like: + // { + // "type": "ImportDeclaration", + // "specifiers": [ + // { + // "type": "ImportDefaultSpecifier", + // "local": { + // "type": "Identifier", + // "name": "EnergySkateParkColorScheme", + // } + // } + // ] + // } + node.specifiers.forEach( specifier => { + context.report( { + node: node, + loc: node.loc, + message: `${specifier.local.name}: import statement should be on a single line.` + } ); } ); - } ); + } } - } - }; + }; + } }; module.exports.schema = [ diff --git a/eslint/rules/uppercase-statics-should-be-readonly.js b/eslint/rules/uppercase-statics-should-be-readonly.js index 756ebfbc..898de05d 100644 --- a/eslint/rules/uppercase-statics-should-be-readonly.js +++ b/eslint/rules/uppercase-statics-should-be-readonly.js @@ -11,18 +11,20 @@ * @copyright 2022 University of Colorado Boulder */ -module.exports = function( context ) { +module.exports = { + create: function( context ) { - return { + return { - PropertyDefinition: node => { - if ( node.key.name && node.key.name === node.key.name.toUpperCase() && node.static && !node.readonly ) { - context.report( { - node: node, - loc: node.loc, - message: `Uppercase static field ${node.key.name} should be readonly` - } ); + PropertyDefinition: node => { + if ( node.key.name && node.key.name === node.key.name.toUpperCase() && node.static && !node.readonly ) { + context.report( { + node: node, + loc: node.loc, + message: `Uppercase static field ${node.key.name} should be readonly` + } ); + } } - } - }; + }; + } }; \ No newline at end of file diff --git a/eslint/rules/visibility-annotation.js b/eslint/rules/visibility-annotation.js index 080e1b01..70fb958d 100644 --- a/eslint/rules/visibility-annotation.js +++ b/eslint/rules/visibility-annotation.js @@ -11,39 +11,41 @@ // Rule Definition //------------------------------------------------------------------------------ -module.exports = function( context ) { - const annotations = [ '@private', '@public', '@protected' ]; - - // these are still MethodDefinition nodes, but don't require an annotation - const exemptMethods = [ 'get', 'set', 'constructor' ]; - - // documentation-based annotations are not required in TypeScript files. - const filenameLowerCase = context.getFilename().toLowerCase(); - const isTypeScriptFile = filenameLowerCase.endsWith( '.ts' ) || filenameLowerCase.endsWith( '.tsx' ); - - return { - MethodDefinition: node => { - if ( !exemptMethods.includes( node.kind ) && !isTypeScriptFile ) { - let includesAnnotation = false; - const commentsBefore = context.getSourceCode().getCommentsBefore( node ); - - // OK as long as any comment above the method (block or line) has an annotation - for ( let i = 0; i < commentsBefore.length; i++ ) { - if ( annotations.some( annotation => commentsBefore[ i ].value.includes( annotation ) ) ) { - includesAnnotation = true; - break; +module.exports = { + create: function( context ) { + const annotations = [ '@private', '@public', '@protected' ]; + + // these are still MethodDefinition nodes, but don't require an annotation + const exemptMethods = [ 'get', 'set', 'constructor' ]; + + // documentation-based annotations are not required in TypeScript files. + const filenameLowerCase = context.getFilename().toLowerCase(); + const isTypeScriptFile = filenameLowerCase.endsWith( '.ts' ) || filenameLowerCase.endsWith( '.tsx' ); + + return { + MethodDefinition: node => { + if ( !exemptMethods.includes( node.kind ) && !isTypeScriptFile ) { + let includesAnnotation = false; + const commentsBefore = context.getSourceCode().getCommentsBefore( node ); + + // OK as long as any comment above the method (block or line) has an annotation + for ( let i = 0; i < commentsBefore.length; i++ ) { + if ( annotations.some( annotation => commentsBefore[ i ].value.includes( annotation ) ) ) { + includesAnnotation = true; + break; + } + } + if ( !includesAnnotation ) { + context.report( { + node: node, + loc: node.loc, + message: `${node.key.name}: Missing visibility annotation.` + } ); } - } - if ( !includesAnnotation ) { - context.report( { - node: node, - loc: node.loc, - message: `${node.key.name}: Missing visibility annotation.` - } ); } } - } - }; + }; + } }; module.exports.schema = [