stock eslint support

[retorquere]

This PR does a few things:

• the bundled processor now works with the stock eslint, making the teslint wrapper superfluous. I think this should also makes vscode-ran eslint work, but I have not tested this
• The header is now found using espree rather than using a regex (this is what failed when I was regex-searching for the header)
• partial disabling of rules to implement different checking regimes for header/testcases, previously done by injecting eslint rules into the linted code, is now relegated to the processor, making for much more maintainable code, and finally
• adds the linebreak-style test, which started all of this

[AbeJellinek]

I think this should also makes vscode-ran eslint work, but I have not tested this

That's always worked for me, but I might be the only one.

Let's keep teslint.js around, though, even if we don't use it for npm run lint. Scaffold relies on the custom options it provides. We can add support for running ESLint directly in the beta, but we shouldn't break Scaffold on Z6.

[adam3smith]

This is great!
Question while we're rewriting this -- is there any way to make this work in feature branches? It's a pain to relint once I'm no longer on master/main

[retorquere]

Publishing the eslint plugin on npmjs should allow that I think.

[retorquere]

Let's keep teslint.js around, though, even if we don't use it for npm run lint. Scaffold relies on the custom options it provides. We can add support for running ESLint directly in the beta, but we shouldn't break Scaffold on Z6.

What options are those?

[retorquere]

What options are those?

The options I see in teslint are:

• --output-json: supported by eslint as --format json --output
• --fix/--no-ignore/--quiet: supported by eslint as-is
• --dump-decorated: not supported, but I only added that to debug the rather messy way I disabled linting of certain sections of code by injecting eslint directives. That's now handled cleanly in the postprocess, and the only decoration now required is enclosing the header in (...), making it a valid standalone expression.

[retorquere]

Publishing the eslint plugin on npmjs should allow that I think.

I could do that but I can imagine Zotero would rather keep this under Zotero control.

[dstillman]

What was "should allow that" referring to?

[retorquere]

Linting on feature branches. But maybe I misunderstood - I thought publishing it would make it easier to lint on local clones, but now I'm not sure why tbh. What was it that you can't do now that you want to be able to do on feature branches?

[AbeJellinek]

--output-json: supported by eslint as --format json --output

Right, but Scaffold passes --output-json, so we need to support --output-json (even if that just means rewriting it to --format json --output).

[adam3smith]

(Sorry if I'm derailing the PR -- please let me know if you want me to move this to a separate issue):

What was it that you can't do now that you want to be able to do on feature branches?

If I try to run the linter locally while I'm on any branch other than master it errors out. So in my local fork (picking one random generally applicable example):

git checkout PhilPapers
npm run lint -- PhilPapers.js

produces

> translators-check@1.0.2 lint
> teslint PhilPapers.js

node:child_process:960
    throw err;
    ^

Error: Command failed: git grep '"lastUpdated"' master *.js

(followed by a stack trace). I assumed this was a general issue? It means I have to do some awkward git-gymnastic when I want to re-lint work on a translator that's in any branch. This is on Windows, in case that matter.

[retorquere]

Oh yeah that was a hopelessly overengineered check, that works on any branch now. wait, I still have to fix this. Git doesn't preserve file mod time.

[retorquere]

Should work now, and teslint is back.

[retorquere]

I think this should also makes vscode-ran eslint work, but I have not tested this

should make vscode-ran eslint fixes that is. But untested. I hardly ever use vscode.

[AbeJellinek]

OK, great. I forgot that in Z7 we no longer use --output-json, but rather pass a - as the output path to indicate outputting JSON to stdout. That's kind of confusing and not a great approach - I should have gone with standard ESLint options - so let's update the Z7 beta concurrently with this. PR: zotero/zotero#3498

[dstillman]

Thanks!

[saschanaz]

It seems the lint job fails now though?

Run ./lint.sh

> translators-check@1.0.2 lint
> eslint ""


Oops! Something went wrong! :(

ESLint: 8.38.0

Error: 'patterns' must be a non-empty string or an array of non-empty strings
    at ESLint.lintFiles (/home/runner/work/translators/translators/node_modules/eslint/lib/eslint/eslint.js:545:19)
    at Object.execute (/home/runner/work/translators/translators/node_modules/eslint/lib/cli.js:417:36)
    at async main (/home/runner/work/translators/translators/node_modules/eslint/bin/eslint.js:135:24)
Error: Process completed with exit code 2.

[dstillman]

There's just nothing to lint for a PR like yours that doesn't touch translators. Should be fixed to exit gracefully, though.

[aborel]

Is this supposed to work if more than one translator has been modified? I have 2 modified files in one of my PRs and there is an error in the lint check that I don't remember seeing before: https://github.com/zotero/translators/actions/runs/7350138469/job/20011290416?pr=3206
(I know that the Selenium test error is a different issue)

[saschanaz]

No files matching the pattern "Zenodo pre-2023.js\nZenodo.js" were found.

\n looks suspicious.

[retorquere]

There's just nothing to lint for a PR like yours that doesn't touch translators. Should be fixed to exit gracefully, though.

Shouldn't lint.sh already detect that and not start the linter?

[saschanaz]

Shouldn't lint.sh already detect that and not start the linter?

That's #3211 which is now fixed.