Bug 1809497 - Separate running of Prettier from ESLint completely. r=…

…linter-reviewers,devtools-reviewers,andi

Differential Revision: https://phabricator.services.mozilla.com/D174867

browser/components/newtab/.eslintrc.js

@@ -2,12 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-let prettierRules = { "prettier/prettier": "error" };
-
-if (process.env.MOZ_SEPARATE_PRETTIER) {
-  prettierRules = { "prettier/prettier": "off" };
-}
-
 module.exports = {
   // When adding items to this file please check for effects on sub-directories.
   parserOptions: {
@@ -118,7 +112,7 @@ module.exports = {
     },
   ],
   rules: {
-    ...prettierRules,
+    "prettier/prettier": "off",
 
     "fetch-options/no-fetch-credentials": "error",
 

devtools/client/debugger/src/.eslintrc.js

@@ -2,12 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at <http://mozilla.org/MPL/2.0/>. */
 
-let prettierRules = { "prettier/prettier": "error" };
-
-if (process.env.MOZ_SEPARATE_PRETTIER) {
-  prettierRules = { "prettier/prettier": "off" };
-}
-
 module.exports = {
   plugins: ["react", "mozilla", "@babel", "prettier", "import", "file-header"],
   globals: {
@@ -53,7 +47,7 @@ module.exports = {
     jest: true,
   },
   rules: {
-    ...prettierRules,
+    "prettier/prettier": "off",
 
     // These are the rules that have been configured so far to match the
     // devtools coding style.

docs/contributing/editors/vscode.rst

@@ -21,7 +21,8 @@ VS Code provides number of extensions for JavaScript, Rust, etc. By default,
 Firefox source tree comes with its own set of recommendations of Visual Studio
 Code extensions. They will be offered when you first open the project.
 
-They are listed in `.vscode/extensions.json <https://searchfox.org/mozilla-central/source/.vscode/extensions.json>`__.
+If you need to refer to them later, the extensions are listed in
+`.vscode/extensions.json <https://searchfox.org/mozilla-central/source/.vscode/extensions.json>`__.
 
 For Rust development, the `rust-analyzer <https://marketplace.visualstudio.com/items?itemName=matklad.rust-analyzer>`__ extension is recommended.
 `See the manual <https://rust-analyzer.github.io/manual.html>`__ for more information.
@@ -73,21 +74,15 @@ Recommended Preferences
 .. note::
 
    These are automatically set when running ``./mach ide vscode`` but may be
-   changed manually.
+   changed manually. These are set only for particular file types.
 
-Code Formatting
-~~~~~~~~~~~~~~~
+* ``"editor.formatOnSave": true``
+    * This will turn on automatically fixing formatting issues when you save a file.
+* ``"editor.defaultFormatter": "esbenp.prettier-vscode"``
+   * This sets the default formatter to prettier using the recommended prettier
+     extension.
 
-This will turn on automatically fixing formatting issues when you save a file:
-
-.. code::
-
-  "editor.formatOnSave": true
-
-File Associations
-~~~~~~~~~~~~~~~~~
-
-``*.jsm`` and ``*.sjs`` file extensions should be associated with JavaScript:
+``*.jsm`` and ``*.sjs`` file extensions should also be associated with JavaScript:
 
 .. code::
 

python/mozbuild/mozbuild/backend/mach_commands.py

@@ -148,6 +148,12 @@ def setup_vscode(command_context, vscode_cmd):
             "*.jsm": "javascript",
             "*.sjs": "javascript",
         },
+        # Note, the top-level editor settings are left as default to allow the
+        # user's defaults (if any) to take effect.
+        "[javascript][javascriptreact][typescript][typescriptreact]": {
+            "editor.defaultFormatter": "esbenp.prettier-vscode",
+            "editor.formatOnSave": True,
+        },
     }
 
     import difflib
@@ -324,7 +330,6 @@ def setup_clangd_rust_in_vscode(command_context):
 
 
 def get_clang_tools(command_context, clang_tools_path):
-
     import shutil
 
     if os.path.isdir(clang_tools_path):

tools/lint/eslint/__init__.py

@@ -107,43 +107,38 @@ def lint(paths, config, binary=None, fix=None, rules=[], setup=None, **lintargs)
         return result
 
     # Then run Prettier
-    if "MOZ_SEPARATE_PRETTIER" in os.environ:
-        patterns = []
-        for p in paths:
-            filename, file_extension = os.path.splitext(p)
-            if file_extension:
-                patterns.append(p)
-            else:
-                patterns.append(
-                    p + "/**/*.+({})".format("|".join(config["extensions"]))
-                )
+    patterns = []
+    for p in paths:
+        filename, file_extension = os.path.splitext(p)
+        if file_extension:
+            patterns.append(p)
+        else:
+            patterns.append(p + "/**/*.+({})".format("|".join(config["extensions"])))
 
-        cmd_args = (
-            [
-                binary,
-                os.path.join(
-                    module_path, "node_modules", "prettier", "bin-prettier.js"
-                ),
-                "--list-different",
-            ]
-            + extra_args
-            # Prettier does not support exclude arguments.
-            # + exclude_args
-            # Prettier only supports this from 2.3 and above (bug 1826062).
-            # + "--no-error-on-unmatched-pattern",
-            + patterns
-        )
-        log.debug("Prettier command: {}".format(" ".join(cmd_args)))
+    cmd_args = (
+        [
+            binary,
+            os.path.join(module_path, "node_modules", "prettier", "bin-prettier.js"),
+            "--list-different",
+        ]
+        + extra_args
+        # Prettier does not support exclude arguments.
+        # + exclude_args
+        # Prettier only supports this from 2.3 and above (bug 1826062).
+        # + "--no-error-on-unmatched-pattern",
+        + patterns
+    )
+    log.debug("Prettier command: {}".format(" ".join(cmd_args)))
 
-        if fix:
-            cmd_args.append("--write")
+    if fix:
+        cmd_args.append("--write")
 
-        prettier_result = run_prettier(cmd_args, config, fix)
-        if prettier_result == 1:
-            return prettier_result
+    prettier_result = run_prettier(cmd_args, config, fix)
+    if prettier_result == 1:
+        return prettier_result
 
-        result["results"].extend(prettier_result["results"])
-        result["fixed"] = result["fixed"] + prettier_result["fixed"]
+    result["results"].extend(prettier_result["results"])
+    result["fixed"] = result["fixed"] + prettier_result["fixed"]
     return result
 
 

tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js

@@ -4,12 +4,6 @@
 
 "use strict";
 
-let prettierRules = { "prettier/prettier": "error" };
-
-if (process.env.MOZ_SEPARATE_PRETTIER) {
-  prettierRules = { "prettier/prettier": "off" };
-}
-
 /**
  * The configuration is based on eslint:recommended config. The details for all
  * the ESLint rules, and which ones are in the recommended configuration can
@@ -124,8 +118,6 @@ module.exports = {
   // When adding items to this file please check for effects on all of toolkit
   // and browser
   rules: {
-    ...prettierRules,
-
     // This may conflict with prettier, so we turn it off.
     "arrow-body-style": "off",
 
@@ -347,6 +339,8 @@ module.exports = {
     // This may conflict with prettier, so turn it off.
     "prefer-arrow-callback": "off",
 
+    "prettier/prettier": "off",
+
     // This generates too many false positives that are not easy to work around,
     // and false positives seem to be inherent in the rule.
     "require-atomic-updates": "off",

tools/lint/test/conftest.py

@@ -117,6 +117,10 @@ def lint(config, root, request):
     This will automatically pass in the 'config' and 'root' arguments if not
     specified.
     """
+
+    if hasattr(request.module, "fixed"):
+        request.module.fixed = 0
+
     try:
         func = findobject(config["payload"])
     except (ImportError, ValueError):

tools/lint/test/files/eslint/testprettierignore

@@ -0,0 +1 @@
+# Intentionally empty file.

tools/lint/test/test_eslint.py

@@ -9,7 +9,13 @@
 @pytest.fixture
 def eslint(lint):
     def inner(*args, **kwargs):
-        kwargs["extra_args"] = ["--no-ignore"]
+        # --no-ignore is for ESLint to avoid the .eslintignore file.
+        # --ignore-path is because Prettier doesn't have the --no-ignore option
+        # and therefore needs to be given an empty file for the tests to work.
+        kwargs["extra_args"] = [
+            "--no-ignore",
+            "--ignore-path=tools/lint/test/files/eslint/testprettierignore",
+        ]
         return lint(*args, **kwargs)
 
     return inner
@@ -37,10 +43,9 @@ def test_bad_import(eslint, config, paths):
     assert results == 1
 
 
-def test_rule(eslint, config, create_temp_file):
+def test_eslint_rule(eslint, config, create_temp_file):
     contents = """var re = /foo   bar/;
-    var re = new RegExp("foo   bar");
-
+var re = new RegExp("foo   bar");
 """
     path = create_temp_file(contents, "bad.js")
     results = eslint(
@@ -50,24 +55,43 @@ def test_rule(eslint, config, create_temp_file):
     assert len(results) == 2
 
 
-def test_fix(eslint, config, create_temp_file):
+def test_eslint_fix(eslint, config, create_temp_file):
     contents = """/*eslint no-regex-spaces: "error"*/
 
-    var re = /foo   bar/;
-    var re = new RegExp("foo   bar");
+var re = /foo   bar/;
+var re = new RegExp("foo   bar");
+
+var re = /foo   bar/;
+var re = new RegExp("foo   bar");
+
+var re = /foo   bar/;
+var re = new RegExp("foo   bar");
+"""
+    path = create_temp_file(contents, "bad.js")
+    eslint([path], config=config, root=build.topsrcdir, fix=True)
+
+    # ESLint returns counts of files fixed, not errors fixed.
+    assert fixed == 1
 
 
-    var re = /foo   bar/;
-    var re = new RegExp("foo   bar");
+def test_prettier_rule(eslint, config, create_temp_file):
+    contents = """var re = /foobar/;
+    var re = "foo";
+"""
+    path = create_temp_file(contents, "bad.js")
+    results = eslint([path], config=config, root=build.topsrcdir)
+
+    assert len(results) == 1
 
-    var re = /foo   bar/;
-    var re = new RegExp("foo   bar");
 
+def test_prettier_fix(eslint, config, create_temp_file):
+    contents = """var re = /foobar/;
+    var re = "foo";
 """
     path = create_temp_file(contents, "bad.js")
     eslint([path], config=config, root=build.topsrcdir, fix=True)
 
-    # ESLint returns counts of files fixed, not errors fixed.
+    # Prettier returns counts of files fixed, not errors fixed.
     assert fixed == 1