Skip to content

Commit

Permalink
Move unfixed rules into the flat eslint config
Browse files Browse the repository at this point in the history 
Rather than having a convoluted system to check unfixed rules,
this just has them apply to all files, and only selectively
disables them for files we know to be unfixed.
This helps us not introduce more issues in the meantime.
I did generalize a bit for release-editor and similar folders
where we don't expect to fix the files but convert them entirely,
so there's no point fixing them file by file.
  • Loading branch information
@reosarevok
reosarevok committed Jul 16, 2024
1 parent 7e75318 commit 30ab720
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 144 deletions.
27 changes: 5 additions & 22 deletions HACKING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,28 +403,11 @@ against all of these rules, run:
Replace `$file_or_directory` with the path to the file or directory you'd like
to check.

If you want to check only a specific rule (say, because you'd like to fix that
particular rule across the codebase and want to ignore others while doing so),
we also have a script for that:

$ ./script/check_eslint_rule $rule $file_or_directory

In this case, you'd replace `$rule` with a string defining the specific rule
you'd like to check, in [levn](https://github.com/gkz/levn) format. For
example, `'block-scoped-var: [warn]'`. Further documentation on how to specify
these can be found
[here](https://eslint.org/docs/user-guide/command-line-interface#--rule).

A second file, [.eslintrc.unfixed.yaml](.eslintrc.unfixed.yaml), lists rules
we want to follow but we don't yet enforce. We also have a script to check a file
or directory against all of these rules,
[script/check_unfixed_eslint_rules](script/check_unfixed_eslint_rules):

$ ./script/check_unfixed_eslint_rules $file_or_directory

You can also check these unfixed rules one by one with `check_eslint_rule`
as indicated above.

Rules that are not yet enforced across all the codebase have their own
sections in [eslint.config.mjs](eslint.config.mjs) disabling them for files
that are not fixed yet. If you want to check the rule across the codebase,
you can temporarily remove that section in the config file. If you fix all the
issues, you can permanently remove the section.

Reports
-------
Expand Down
187 changes: 187 additions & 0 deletions eslint.config.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ export default [
// Possible Errors
'for-direction': 'error',
'no-async-promise-executor': 'error',
'no-await-in-loop': 'error',
'no-cond-assign': 'warn',
'no-constant-condition': ['error', {checkLoops: false}],
'no-control-regex': 'off',
Expand Down Expand Up @@ -155,19 +156,62 @@ export default [

// Best Practices
'block-scoped-var': 'warn',
'camelcase': ['warn', {
properties: 'never',
allow: [
'l_admin',
'ln_admin',
'l_attributes',
'ln_attributes',
'lp_attributes',
'l_countries',
'ln_countries',
'lp_countries',
'l_history',
'l_instrument_descriptions',
'ln_instrument_descriptions',
'lp_instrument_descriptions',
'l_instruments',
'ln_instruments',
'lp_instruments',
'l_languages',
'ln_languages',
'lp_languages',
'l_relationships',
'ln_relationships',
'lp_relationships',
'l_scripts',
'ln_scripts',
'lp_scripts',
'l_statistics',
'ln_statistics',
'lp_statistics',
'N_l',
'N_ln',
'N_lp',
'N_l_statistics',
'N_lp_statistics',
'__webpack_public_path__',
],
}],
'class-methods-use-this': 'off',
'consistent-return': 'error',
'consistent-this': ['warn', 'self'],
'curly': ['error', 'all'],
'dot-location': ['warn', 'property'],
'dot-notation': ['warn', {allowKeywords: true}],
'eqeqeq': ['warn', 'smart'],
'no-alert': 'off',
'no-else-return': 'warn',
'no-eq-null': 'off',
'no-floating-decimal': 'warn',
'no-global-assign': 'error',
'no-multi-spaces': ['error', {ignoreEOLComments: true}],
'no-useless-catch': 'warn',
'no-var': 'warn',
'prefer-const': 'warn',
'radix': 'warn',
'sort-keys': ['warn', 'asc', {caseSensitive: false, natural: true}],

// Strict Mode
'strict': 'off',
Expand Down Expand Up @@ -273,6 +317,7 @@ export default [
}],
'switch-colon-spacing': ['warn', {after: true, before: false}],
'template-tag-spacing': ['warn', 'never'],
'wrap-iife': 'warn',

// ECMAScript 6
'constructor-super': 'error',
Expand Down Expand Up @@ -323,6 +368,8 @@ export default [
'react/forbid-elements': 'off',
'react/forbid-prop-types': 'off',
'react/forbid-foreign-prop-types': 'off',
'react/jsx-no-bind': ['warn', {ignoreDOMComponents: true}],
'react/no-access-state-in-setstate': 'error',
'react/no-array-index-key': 'off',
'react/no-children-prop': 'error',
'react/no-danger': 'off',
Expand All @@ -337,7 +384,9 @@ export default [
'react/no-direct-mutation-state': 'error',
'react/no-find-dom-node': 'warn',
'react/no-is-mounted': 'warn',
'react/no-multi-comp': ['warn', {ignoreStateless: true}],
'react/no-redundant-should-component-update': 'error',
'react/no-render-return-value': 'warn',
'react/no-set-state': 'off',
'react/no-typos': 'error',
'react/no-string-refs': 'warn',
Expand Down Expand Up @@ -476,4 +525,142 @@ export default [
}],
},
},
/*
* The following sections disable rules we want to enforce, but haven't
* fixed everywhere, only in files with existing issues. This allows us
* to enforce them elsewhere in the meantime.
* If you fix one rule in any of the files in these sections, please remove
* the file from the list; if you fix the last file, remove the section.
*/
{
files: [
'root/static/scripts/edit/components/withLoadedTypeInfo.js',
'root/static/scripts/release/components/ReleaseRelationshipEditor.js',
],
rules: {
'no-await-in-loop': 'off',
},
},
{
files: [
'root/static/scripts/common/MB/Control/Autocomplete.js',
'root/static/scripts/edit/MB/CoverArt.js',
'root/static/scripts/guess-case/MB/Control/GuessCase.js',
'root/static/scripts/jquery.flot.musicbrainz_events.js',
'root/static/scripts/release-editor/**/*',
'root/static/scripts/timeline.js',
],
rules: {
'eqeqeq': 'off',
},
},
{
files: [
'root/static/scripts/jquery.flot.musicbrainz_events.js',
'root/static/scripts/timeline.js',
],
rules: {
'wrap-iife': 'off',
},
},
{
files: [
'root/static/scripts/common/MB/**/*',
'root/static/scripts/edit/MB/**/*',
],
rules: {
'consistent-this': 'off',
},
},
{
files: [
'root/static/scripts/common/MB/**/*',
'root/static/scripts/common/artworkViewer.js',
'root/static/scripts/common/coverart.js',
'root/static/scripts/common/utility/request.js',
'root/static/scripts/edit/MB/**/*',
'root/static/scripts/edit/forms.js',
'root/static/scripts/guess-case/MB/Control/GuessCase.js',
'root/static/scripts/jquery.flot.musicbrainz_events.js',
'root/static/scripts/release-editor/**/*',
'root/static/scripts/series/edit.js',
'root/static/scripts/timeline.js',
],
rules: {
'no-var': 'off',
},
},
{
files: [
'root/static/scripts/common/components/Modal.js',
'root/static/scripts/edit/ExampleRelationships.js',
],
rules: {
'prefer-const': 'off',
},
},
{
files: [
'root/server/components.mjs',
'root/static/scripts/common/MB.js',
'root/static/scripts/common/MB/**/*',
'root/static/scripts/common/artworkViewer.js',
'root/static/scripts/common/components/ButtonPopover.js',
'root/static/scripts/edit/MB/**/*',
'root/static/scripts/edit/forms.js',
'root/static/scripts/edit/utility/editDiff.js',
'root/static/scripts/guess-case/MB/Control/GuessCase.js',
'root/static/scripts/jquery.flot.musicbrainz_events.js',
'root/static/scripts/relationship-editor/components/RelationshipDialogContent.js',
'root/static/scripts/release-editor/**/*',
'root/static/scripts/release/components/BatchCreateWorksDialog.js',
'root/static/scripts/series/edit.js',
'root/static/scripts/timeline.js',
'root/utility/activeSanitizedEditor.mjs',
],
rules: {
'sort-keys': 'off',
},
},
{
files: [
'root/static/scripts/common/components/TagEditor.js',
'root/static/scripts/edit/externalLinks.js',
],
rules: {
'react/no-access-state-in-setstate': 'off',
},
},
{
files: [
'root/static/scripts/edit/externalLinks.js',
],
rules: {
'react/no-multi-comp': 'off',
},
},
{
files: [
'root/search/components/ArtistResults.js',
'root/search/components/InstrumentResults.js',
'root/search/components/RecordingResults.js',
'root/search/components/ReleaseResults.js',
'root/search/components/WorkResults.js',
'root/static/scripts/account/components/EditProfileForm.js',
'root/static/scripts/account/components/RegisterForm.js',
'root/static/scripts/common/components/TagEditor.js',
'root/static/scripts/edit/check-duplicates.js',
'root/static/scripts/edit/components/ExternalLinkAttributeDialog.js',
'root/static/scripts/edit/components/FormRowNameWithGuessCase.js',
'root/static/scripts/edit/components/FormRowSelectList.js',
'root/static/scripts/edit/components/ReleaseMergeStrategy.js',
'root/static/scripts/edit/components/URLInputPopover.js',
'root/static/scripts/edit/components/UrlRelationshipCreditFieldset.js',
'root/static/scripts/edit/externalLinks.js',
'root/static/scripts/relationship-editor/components/DialogPreview.js',
],
rules: {
'react/jsx-no-bind': 'off',
},
},
];
83 changes: 0 additions & 83 deletions script/.check_eslint_rule_config.yaml
View file

This file was deleted.

9 changes: 0 additions & 9 deletions script/check_eslint_rule
View file

This file was deleted.

30 changes: 0 additions & 30 deletions script/check_unfixed_eslint_rules
View file

This file was deleted.

0 comments on commit 30ab720

Please sign in to comment.