Closed Bug 1736559 Opened 1 month ago Closed 1 month ago

Codespell incorrectly changes package.json

Categories

(Firefox Build System :: Lint and Formatting, task, P2)

task

Tracking

(firefox95 fixed)

RESOLVED FIXED
95 Branch
Tracking Status
firefox95 --- fixed

People

(Reporter: standard8, Assigned: ahal)

Details

Attachments

(2 files)

I just caught this on a patch that someone had posted.

Running ./mach lint -l codespell --fix causes a change to tools/lint/eslint/eslint-plugin-mozilla/package-lock.json as per below. These files should never be automatically changed by linters.

diff --git a/tools/lint/eslint/eslint-plugin-mozilla/package-lock.json b/tools/lint/eslint/eslint-plugin-mozilla/package-lock.json
index 36be00fdd1967..2d8bc6aeaf069 100644
--- a/tools/lint/eslint/eslint-plugin-mozilla/package-lock.json
+++ b/tools/lint/eslint/eslint-plugin-mozilla/package-lock.json
@@ -1499,7 +1499,7 @@
     "require-from-string": {
       "version": "2.0.2",
       "resolved": "https://registry.npmjs.org/require-from-string/-/require-from-string-2.0.2.tgz",
-      "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/vEw==",
+      "integrity": "sha512-Xf0nWe6RseziFMu+Ap9biiUbmplq6S9/p+7w7YXP/JBHhrUDDUhwa+vANyubuqfZWTveU//DYVGsDG7RKL/view==",
       "dev": true
     },
     "resolve-from": {

I can't figure out how to stop this happening, I've added various variants of package-lock.json (with ** and with full paths) to the exclude list in tools/lint/codespell.yml and it doesn't prevent the change.

Any ideas Andrew?

Flags: needinfo?(ahal)

Adding "**/package-lock.json" seems to do it for me. Note it needs to be in quotes though since * is a special character in yaml.

Assignee: nobody → ahal
Status: NEW → ASSIGNED
Flags: needinfo?(ahal)

We have a check in the 'CodespellProcess' class to ignore errors that are
fixing single letter camelCase errors (since these tend to be used frequently.

Unfortunately, when using '--fix', these errors are fixed regardless as the
fixing happens with the codespell process. Since we increment the 'fix'
variable after the check happens, we don't even report that anything was
'fixed'.

Ideally we would find a way to prevent these types of errors from being fixed,
but for now this at will at least ensure that the user is notified that
something was modified.

Note there's still a bug in the codespell linter though. Basically this check is useless with --fix:
https://searchfox.org/mozilla-central/rev/36aa22c7ea92bd3cf7910774004fff7e63341cf5/tools/lint/spell/__init__.py#58

This check prevents the aforementioned package-lock.json issue from being reported without --fix.

(In reply to Andrew Halberstadt [:ahal] from comment #2)

Adding "**/package-lock.json" seems to do it for me. Note it needs to be in quotes though since * is a special character in yaml.

Hmm, I wonder if I maybe changed the file in the wrong repo... oops.

(In reply to Andrew Halberstadt [:ahal] from comment #5)

Note there's still a bug in the codespell linter though. Basically this check is useless with --fix:
https://searchfox.org/mozilla-central/rev/36aa22c7ea92bd3cf7910774004fff7e63341cf5/tools/lint/spell/__init__.py#58

This check prevents the aforementioned package-lock.json issue from being reported without --fix.

We could use bug 1736560 to fix that maybe.

Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f42752e51c9c
[lint] Fix codespell silently fixing files, r=linter-reviewers,sylvestre
https://hg.mozilla.org/integration/autoland/rev/bd985949aa5c
[lint] Ignore all 'package-lock.json' files in codespell linter, r=linter-reviewers,sylvestre
Status: ASSIGNED → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 95 Branch
You need to log in before you can comment on or make changes to this bug.