Skip to content

Commit

Permalink
report lines for everything by default, use codeTokens only for "code…
Browse files Browse the repository at this point in the history
… only", #791
  • Loading branch information
zepumph committed Nov 7, 2019
1 parent eb345b0 commit 65d0af4
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 42 deletions.
18 changes: 9 additions & 9 deletions eslint/rules/bad-sim-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ module.exports = function( context ) {
const forbiddenTextObjects = [

// babel doesn't support compiling static getters, see https://github.com/phetsims/tasks/issues/983
{ id: ' static get ', codeOnly: true, codeTokens: [ 'static', 'get' ] },
{ id: ' static get ', codeTokens: [ 'static', 'get' ] },

// should be using dot.Util.roundSymmetric, Math.round does not treat positive and negative numbers
// symmetrically see https://github.com/phetsims/dot/issues/35#issuecomment-113587879
{ id: 'Math.round(', codeOnly: true, codeTokens: [ 'Math', '.', 'round', '(' ] },
{ id: 'Math.round(', codeTokens: [ 'Math', '.', 'round', '(' ] },

// should be using `phet.joist.random`
{ id: 'Math.random()', codeOnly: true, codeTokens: [ 'Math', '.', 'random', '(', ')' ] },
{ id: '_.shuffle(', codeOnly: true, codeTokens: [ '_', '.', 'shuffle', '(' ] },
{ id: '_.sample(', codeOnly: true, codeTokens: [ '_', '.', 'sample', '(' ] },
{ id: '_.random(', codeOnly: true, codeTokens: [ '_', '.', 'random', '(' ] },
{ id: 'new Random()', codeOnly: true, codeTokens: [ 'new', 'Random', '(', ')' ] },
{ id: 'Math.random()', codeTokens: [ 'Math', '.', 'random', '(', ')' ] },
{ id: '_.shuffle(', codeTokens: [ '_', '.', 'shuffle', '(' ] },
{ id: '_.sample(', codeTokens: [ '_', '.', 'sample', '(' ] },
{ id: '_.random(', codeTokens: [ '_', '.', 'random', '(' ] },
{ id: 'new Random()', codeTokens: [ 'new', 'Random', '(', ')' ] },

// IE doesn't support:
{ id: 'Number.parseInt(', codeOnly: true, codeTokens: [ 'Number', '.', 'parseInt', '(' ] },
{ id: 'Array.prototype.find', codeOnly: true, codeTokens: [ 'Array', '.', 'prototype', '.', 'find' ] }
{ id: 'Number.parseInt(', codeTokens: [ 'Number', '.', 'parseInt', '(' ] },
{ id: 'Array.prototype.find', codeTokens: [ 'Array', '.', 'prototype', '.', 'find' ] }

// DOT/Util.toFixed or DOT/Util.toFixedNumber should be used instead of toFixed.
// JavaScript's toFixed is notoriously buggy. Behavior differs depending on browser,
Expand Down
7 changes: 4 additions & 3 deletions eslint/rules/bad-text.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,16 @@ module.exports = function( context ) {

// 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',

'@return '
'@return ',

// TODO: uncomment once ESP(B) lint errors fixed, see https://github.com/phetsims/energy-skate-park/issues/140
// { id: ' => { return ', global: true } // if on a one line arrow function returning something, prefer instead `() => theReturn`, see https://github.com/phetsims/chipper/issues/790
// if on a one line arrow function returning something, prefer instead `() => theReturn`, see https://github.com/phetsims/chipper/issues/790
' => { return '
];

return {
Expand Down
62 changes: 32 additions & 30 deletions eslint/rules/getBadTextTester.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module.exports = ( badTexts, context ) => {
return node => {
const sourceCode = context.getSourceCode();
const codeTokens = sourceCode.getTokens( node );
const commentTokens = sourceCode.getAllComments();
const codeLines = sourceCode.lines;
const text = sourceCode.text;

/**
Expand All @@ -33,45 +33,50 @@ module.exports = ( badTexts, context ) => {
*/
const testBadText = forbiddenText => {

// no need to iterate through tokens if it isn't anywhere in the source code
// no need to iterate through lines if the bad text isn't anywhere in the source code
if ( text.indexOf( forbiddenText.id ) >= 0 ) {

// If marked as global, then report the global failure based on the source code indexOf check
if ( forbiddenText.global ) {
context.report( {
node: node,
message: 'File contains bad text: \'' + forbiddenText.id + '\''
} );
// If codeTokens are provided, only test this bad text in code, and not anywhere else.
if ( forbiddenText.codeTokens ) {
testCodeTokens( context, codeTokens, forbiddenText );
}
else {

// search through the tokenized code for the forbidden code tokens, like [ 'Math', '.', 'round' ],
testCodeTokens( context, codeTokens, forbiddenText );

// look through comments
!forbiddenText.codeOnly && commentTokens.forEach( token => {
if ( token.value.indexOf( forbiddenText.id ) >= 0 ) {
context.report( {
loc: token.loc.start,
message: `bad comment text: "${forbiddenText.id}"`
} );
}
} );

// TODO: support REGEX
// if ( forbiddenText.regex instanceof RegExp && forbiddenText.regex.test( token.value ) ) {
// failedText = forbiddenText.id;
// }

// test each line for the presence of the bad text
for ( let i = 0; i < codeLines.length; i++ ) {
const columnIndex = codeLines[ i ].indexOf( forbiddenText.id );
if ( columnIndex >= 0 ) {

// lines are 1 based, codeLines array is 0 based
const badLine = i + 1;

// esprima Token loc object, see https://esprima.readthedocs.io/en/latest/lexical-analysis.html
const loc = {
start: { line: badLine, column: columnIndex },
end: { line: badLine, column: columnIndex + forbiddenText.id.length }
};

context.report( {
node: node,
loc: loc,
message: 'Line contains bad text: \'' + forbiddenText.id + '\''
} );
}
}
}
}
};

badTexts.forEach( badText => {
if ( typeof badText === 'string' ) {
badText = { id: badText, codeTokens: [ badText ] };
badText = { id: badText };
}
assert( typeof badText.id === 'string', 'id required' );
assert( Array.isArray( badText.codeTokens ) || badText.global, 'code tokens or global flag expected' );
testBadText( badText );
} );
};
Expand All @@ -81,13 +86,10 @@ module.exports = ( badTexts, context ) => {
* @property {string} id - the "string-form" id of the bad text. should occur in the source code. Also what is
* displayed on error. Used when checking for bad text in comments.
* @property {Array.<string>} [codeTokens] - a list of the tokenized, ordered code sections that make up the bad text
* within the javascript code (not used for checking comments). If there
* is only one codeToken, then it will also be checked as a substring of each
* code tokens. Required unless specifying "global".
* @property {boolean} [codeOnly] - if true, this object will not be checked for in comments, only in code.
* @property {boolean} [global] - if true, then ignore comment and code token checks, and just error out based on the
* presence of the `id` in the source code for the file. If provided, then codeTokens
* is not needed
* within the javascript code (not used for checking comments). If there
* is only one codeToken, then it will also be checked as a substring of each
* code tokens. Required unless specifying "global". If this is provided,
* then the bad text will only be checked in code, and not via each line.
*/
};

Expand Down

0 comments on commit 65d0af4

Please sign in to comment.