Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update standard lint rules #1276

Closed
samreid opened this issue Jul 11, 2022 · 5 comments
Closed

Update standard lint rules #1276

samreid opened this issue Jul 11, 2022 · 5 comments

Comments

@samreid
Copy link
Member

samreid commented Jul 11, 2022

In #1275 we updated to a more recent version of ESLint. A comment in our .eslintrc file says:

  // The rules are organized like they are in the list at https://eslint.org/docs/rules/
  // First by type, then alphabetically within type
  // Explicitly list all rules so it is easy to see what's here and to keep organized

However, since https://eslint.org/docs/rules/ has been rearranged, we are out of sync. So it would be good to update this.

Afterwards, it would be good to apply the same philosophy (list all rules, even if marked as "off") to the typescript rules. Heads up to @jessegreenberg that I plan to be working on that soon.

The updated lint rules catch these issues:

~/apache-document-root/main/perennial$ grunt lint-everything
Running "lint-everything" task

/Users/samreid/apache-document-root/main/masses-and-springs/js/lab/view/PeriodTraceNode.js
   95:26  error  Unexpected constant truthiness on the left-hand side of a `&&` expression  no-constant-binary-expression
  139:22  error  Unexpected constant truthiness on the left-hand side of a `&&` expression  no-constant-binary-expression

/Users/samreid/apache-document-root/main/perennial-alias/js/grunt/checkoutShas.js
  33:15  error  Unexpected constant truthiness on the left-hand side of a `&&` expression  no-constant-binary-expression

/Users/samreid/apache-document-root/main/perennial/js/grunt/checkoutShas.js
  33:15  error  Unexpected constant truthiness on the left-hand side of a `&&` expression  no-constant-binary-expression

/Users/samreid/apache-document-root/main/rosetta/js/routeHandlers.js
  480:11  error  Use 'Object.hasOwn()' instead of 'Object.prototype.hasOwnProperty.call()'  prefer-object-has-own

/Users/samreid/apache-document-root/main/scenery/js/display/Instance.js
  104:27  error  Unexpected constant truthiness on the left-hand side of a `||` expression  no-constant-binary-expression

✖ 6 problems (6 errors, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

Warning: 6 errors and 0 warnings Use --force to continue.

Aborted due to warnings.

I'll commit proposed fixes and assign responsible devs for any necessary review.

@samreid samreid self-assigned this Jul 11, 2022
samreid added a commit to phetsims/perennial that referenced this issue Jul 11, 2022
samreid added a commit to phetsims/masses-and-springs that referenced this issue Jul 11, 2022
samreid added a commit to phetsims/rosetta that referenced this issue Jul 11, 2022
samreid added a commit to phetsims/scenery that referenced this issue Jul 11, 2022
@samreid
Copy link
Member Author

samreid commented Jul 11, 2022

I pushed the revised rules list and proposed fixes for the failing cases. Assigning repo responsible devs or file authors to take a look at the fixes above.

@jonathanolson
Copy link
Contributor

Looks good, fixed the lazy initialization case in Scenery above (and removed the comment).

@jonathanolson jonathanolson removed their assignment Jul 13, 2022
@jbphet
Copy link
Contributor

jbphet commented Jul 25, 2022

The change to rosetta looks perfectly reasonable to me. Unassigning.

@jbphet jbphet removed their assignment Jul 25, 2022
@chrisklus
Copy link
Contributor

Changes in MAS look great, thanks @samreid. Assuming this is all set to close but I'll check with @samreid since you opened the issue.

@chrisklus chrisklus assigned samreid and unassigned chrisklus Aug 29, 2022
@samreid
Copy link
Member Author

samreid commented Aug 29, 2022

Thanks, closing.

@samreid samreid closed this as completed Aug 29, 2022
zepumph pushed a commit to phetsims/perennial that referenced this issue Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants