Issue #3432261 by jonathan1055, fjgarlin, alexpott, RoSk0: Eslint on contrib...

Issue #3432261 by jonathan1055, fjgarlin, alexpott, RoSk0: Eslint on contrib is making incorrect recommendations

CHANGELOG.md

 # GitLab Templates Changelog
 
+## 1.3.5 - 2024-04-08
+
+#3432261 - Eslint on contrib is making incorrect recommendations.\
+#3427357 - Move default phpcs configuration file and clean up unneeded lines in it.\
+#3435899 - Only update documentation when merging into default branch.\
+#3409296 - Add process for defining dynamic internal variables.\
+#3400979 - Document additional options for phpunit deprecations.\
+#3437578 - Add some missing extensions for PHP files.\
+#3436889 - D7 Fix variable name used for max PHP version.
+
 ## 1.3.4 - 2024-04-03
 
 #3375459 - Cater for version constraints in _TARGET_CORE value.\
 #3400979 - Document SYMFONY_DEPRECATIONS_HELPER options.\
-#3436889 - D7 Allow modules to opt in to testing against max PHP version: OPT_IN_TEST_MAX_PHP.\
+#3436889 - D7 Allow modules to opt in to testing against max PHP version.\
 #3437578 - PHPStan job fails while there is no PHP files to scan.\
 #3437794 - Bump core stable to 10.2.5.\
 #3436206 - Add lorem-ipsum spelling dictionary.

docs/jobs/eslint.md

 # ESLint
 
-If the module has `.js` or `.yml` files, the [eslint](https://eslint.org/) too will run against them.
+If the module has `.js` or `.yml` files, then [eslint](https://eslint.org/) will be run to validate coding standards for these files.
 
-You can use `.prettierrc.json` or `.prettierignore` files to configure the behavior of this job.
+Your project does not need a `.eslintrc.json` file because a default one matching Core's standards `core/.eslintrc.passing.json` is used. If you do create a `.eslintrc.json` file then it only needs to contain the specific rules that you want to override, because all other rules will be inherited due to ESLint's cascading configuration.
 
-If there are errors found in the job, you will get a useful artifact named `_eslint.patch` which you could run against your module's code to fix the issues that were found.
+You can pass additional options to the `eslint` call via the `_ESLINT_EXTRA` variable. This variable has a default value of `--quiet` to match what Drupal core does.
+
+If there are `.js` or `.yml` files that you do not want to fix, or cannot fix for whatever reason, these can be ignored by adding the names to a `.eslintignore` file stored in your project top-level folder. Files and paths listed here will be completely ignored for both ESLint and Prettier checking. An example where this would be needed is if you have minified javascript files.
+```
+# Ignore all minified javascript files
+**/*.min.js
+```
+
+## Prettier
+
+"Prettier" processing is the part of ESLint that is concerned with formatting and file layout, for both `.js` and `.yml` files. Formatting rules follow the definitions used by Drupal Core as defined in `core/.prettierrc.json`. This is the default, but projects can have their own definitions in a `.prettierrc.json` file which will be used instead of the core rules. Unlike ESLint, the Prettier rules do not use cascading configuration, so if you do have a `.prettierrc.json` file it must contain all the rules that you want your project to follow.
+
+If there are files that you do not want to fix, or cannot fix, for Prettier formatting, these can be ignored by adding the file paths to a `.prettierignore` file. An example where this is needed is the Config and Views files exported from UI, as these do not follow the Prettier formatting rules. So rather than manually edit the exported files, they can be ignored for Prettier formatting (but still be checked for ESLint standards).
+```
+config/*/views.*.yml
+tests/modules/*/config/install/*.yml
+```
+
+## Fixing the reported problems
+
+If there are standards and formatting errors that can be fixed automatically, you will get a useful artifact named `_eslint.patch` which you can run against your module's code to fix the issues that were found.

includes/include.drupalci.main.yml

@@ -585,28 +585,33 @@ eslint:
   script:
     # Change directory to the project root folder
     - cd $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/$CI_PROJECT_NAME
-    # Configure prettier
+    # Configure eslint with core defaults. We use core/.eslintrc.passing.json which includes core/.eslintrc.json and .eslintrc.jquery.json.
+    # These links are created in the folder above modules/custom/$CI_PROJECT_NAME and will be used in addition to the project's own .eslintrc.json.
+    - ln -s $CI_PROJECT_DIR/$_WEB_ROOT/core/.eslintrc.passing.json $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/.eslintrc.json
+    - ln -s $CI_PROJECT_DIR/$_WEB_ROOT/core/.eslintrc.jquery.json $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/.eslintrc.jquery.json
+    # Configure prettier with core defaults.
     - test -e .prettierrc.json || ln -s $CI_PROJECT_DIR/$_WEB_ROOT/core/.prettierrc.json .
     - test -e .prettierignore || echo '*.yml' > .prettierignore
     - echo "ESLINT version $(${CI_PROJECT_DIR}/${_WEB_ROOT}/core/node_modules/.bin/eslint --version)"
     # The first run of eslint generates a junit output file.
     # The `|| EXIT_CODE_FILE=$?` stores an exit code if the job fails, and makes sure the script continues.
     # https://stackoverflow.com/questions/59180675/how-to-continue-job-even-when-script-fails
-    - $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml --format=junit --output-file=$CI_PROJECT_DIR/junit.xml . || EXIT_CODE_FILE=$?
+    - $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml --format=junit --output-file=$CI_PROJECT_DIR/junit.xml $_ESLINT_EXTRA . || EXIT_CODE_FILE=$?
     # The artifact file cannot be optional, it must exist or the job gives a failure exit code. Therefore create empty patch file in case there are no fixes.
     - touch $CI_PROJECT_DIR/_eslint.patch
-    # If there are no eslint errors or warnings then exit early as there is no need to try the --fix option or create a patch.
+    # Exit early if we cannot --fix anything. This also caters for config failures.
     - |
-      if [ "$EXIT_CODE_FILE" == "" ]; then
-        printf "$DIVIDER\nThere are no ESLINT errors or warnings$DIVIDER\n"
-        exit
+      if [ "$EXIT_CODE_FILE" != "1" ]; then
+        [[ "$EXIT_CODE_FILE" == "" ]] && printf "$DIVIDER\nThere are no ESLINT errors or warnings$DIVIDER\n"
+        echo "Exiting with EXIT_CODE=$EXIT_CODE_FILE"
+        exit $EXIT_CODE_FILE
       fi
     # There are eslint warnings so run it a second time to write the messages to the log.
     - printf "$DIVIDER\nThese are the current ESLINT errors and warnings$DIVIDER\n"
-    - $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml . || true
+    - $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml $_ESLINT_EXTRA . || true
     # Run a third time with the --fix option to enable a patch to be created.
     - printf "$DIVIDER\nNow running ESLINT using the --fix option. Any errors shown below are not fixable automatically.$DIVIDER\n"
-    - $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml --fix . || EXIT_CODE_FIX=$?
+    - $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml $_ESLINT_EXTRA --fix . || EXIT_CODE_FIX=$?
     # Get the names of the files that have been changed by using the --fix option.
     # Use -G"." to ignore files with no actual content changes, as there may be files where the only change is the mode and we do not want these.
     # Also limit to just changes in .js and .yml files, as other files such as composer.json may have been changed earlier, not by eslint --fix.
@@ -619,6 +624,8 @@ eslint:
       else
         printf "$DIVIDER\nNothing could be fixed with the --fix option$DIVIDER\n"
       fi
+    - test -f .eslintignore && echo "===  This is .eslintignore  ===" && cat .eslintignore || true
+    - test -f .prettierignore && echo "=== This is .prettierignore ===" && cat .prettierignore || true
     - printf "Exiting with EXIT_CODE=$EXIT_CODE_FILE\n"
     - exit $EXIT_CODE_FILE
   allow_failure: true

includes/include.drupalci.variables.yml

@@ -96,6 +96,10 @@ variables:
     value: ""
     description: "Additional options that are appended to the `CSpell` job call."
 
+  _ESLINT_EXTRA:
+    value: "--quiet"
+    description: "Additional options that are appended to the `eslint` job call. The default value `--quiet` matches what Drupal core `eslint` job does. You can set it to empty to get warnings or add any additional options that you might need for the `eslint` job."
+
   _SHOW_ENVIRONMENT_VARIABLES:
     value: "0"
     description: "Set to 1 to show all the environment variables in the Composer and PhpUnit jobs. Known variables with Personal Identifiable Information will still be hidden. The default is 0 for no output at all."