Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eslint: no-undef: also triggers for builtins and undefined assignment #3374

Closed
jelly opened this issue May 21, 2024 · 7 comments · Fixed by #3973 or #4209
Closed

eslint: no-undef: also triggers for builtins and undefined assignment #3374

jelly opened this issue May 21, 2024 · 7 comments · Fixed by #3973 or #4209
Assignees
Labels
C-bug Category - Bug

Comments

@jelly
Copy link
Contributor

jelly commented May 21, 2024

The rule defined here for eslint triggers for builtins such as String, parseInt, JSON, Array all should be available.

Relevant eslintrc.json config:

    "env": {
        "browser": true,
        "es2022": true
    },

So I haven't seen warnings for console being undefined.

  ⚠ eslint(no-undef): Disallow the use of undeclared variables.
     ╭─[src/components/create-vm-dialog/createVmDialog.jsx:184:38]
 183 │             return "";
 184 │         tmpRetName = retName + '-' + String.fromCharCode(i);
     ·                                      ──────
 185 │     }
     ╰────
  help: 'String' is not defined.

And for assignment where the a variable is assigned to undefined.

  ⚠ eslint(no-undef): Disallow the use of undeclared variables.
      ╭─[src/components/create-vm-dialog/createVmDialog.jsx:1145:69]
 1144 │                 [key]: value,
 1145 │                 storageVolume: storageVolume ? storageVolume.name : undefined,
      ·                                                                     ─────────
 1146 │             });
      ╰────
  help: 'undefined' is not defined.
@jelly
Copy link
Contributor Author

jelly commented Jul 11, 2024

Just re-checked it with 0.6.0:

  × eslint(no-undef): Disallow the use of undeclared variables.
      ╭─[src/ImageRunModal.jsx:1130:55]
 1129 │                         </FormGroup>
 1130 │                         {version.localeCompare("4.3", undefined, { numeric: true, sensitivity: 'base' }) >= 0 &&
      ·                                                       ─────────
 1131 │                         <FormGroup isInline hasNoPaddingTop fieldId='run-image-healthcheck-action' label={_("When unhealthy") }
      ╰────
  help: 'undefined' is not defined.

  × eslint(no-undef): Disallow the use of undeclared variables.
     ╭─[src/ImageRunModal.jsx:396:68]
 395 │                             if (result.status === "fulfilled") {
 396 │                                 imageResults = imageResults.concat(JSON.parse(result.value));
     ·                                                                    ────

And still fails for parseInt, Date, Object, Math, encodeURIComponent, Number.

@Boshen Boshen reopened this Jul 11, 2024
@jelly
Copy link
Contributor Author

jelly commented Jul 11, 2024

I can't reproduce this by adding Date to the pass array of no-undef, so I guess this might also have to do with running oxlint --config .eslintrc.json

@jelly
Copy link
Contributor Author

jelly commented Jul 11, 2024

[jelle@t14s][~/projects/cockpit-podman]%oxlint   -A all -D no-undef src/ImageRunModal.jsx

  × eslint(no-undef): Disallow the use of undeclared variables.
     ╭─[src/ImageRunModal.jsx:586:21]
 585 │                 .catch(err => {
 586 │                     console.warn("Failed to start podman-restart.service:", JSON.stringify(err));
     ·                     ───────
 587 │                 });
     ╰────
  help: 'console' is not defined.

  × eslint(no-undef): Disallow the use of undeclared variables.
     ╭─[src/ImageRunModal.jsx:838:53]
 837 │                                 {...(this.state.searchInProgress && { loadingVariant: 'spinner' })}
 838 │                                 menuAppendTo={() => document.body}
     ·                                                     ────────
 839 │                                 variant={SelectVariant.typeahead}
     ╰────
  help: 'document' is not defined.

Finished in 22ms on 1 file with 1 rules using 8 threads.
Found 0 warnings and 2 errors.

So this has something to do with our eslintrc.json. I'll try to investigate this further, as this seems to be an eslint config issue, shall I open a new issue?

https://github.com/cockpit-project/cockpit-podman/blob/main/.eslintrc.json

@jelly
Copy link
Contributor Author

jelly commented Jul 11, 2024

    "env": {
        "browser": true,
        "es2022": true
    },

So my theory is that oxlint treats this as es2022 only, while eslint does what one might expect, es2022 and lower. Seeing that the JS globals are indexed as an map with keys es2022 etc.

And indeed, changing the configuration to:

    "env": {
        "browser": true,
        "builtin": true,
        "node": true,
        "es2022": true
    },

Makes oxlint pass and works as eslint.

@jelly
Copy link
Contributor Author

jelly commented Jul 11, 2024

All that's left now seems to be:

[jelle@t14s][~/projects/cockpit-machines]%~/projects/oxc/target/debug/oxlint --config .eslintrc.json src

  × eslint(no-undef): Disallow the use of undeclared variables.
     ╭─[src/helpers.js:172:38]
 171 │     if (window.debugging === "all" || window.debugging?.includes("machines"))
 172 │         console.debug.apply(console, arguments);
     ·                                      ─────────
 173 │ }
     ╰────
  help: 'arguments' is not defined.

That's not in the GLOBALS in eslint so I am not sure how they detect that.

@rzvxa
Copy link
Contributor

rzvxa commented Jul 11, 2024

All that's left now seems to be:

[jelle@t14s][~/projects/cockpit-machines]%~/projects/oxc/target/debug/oxlint --config .eslintrc.json src

  × eslint(no-undef): Disallow the use of undeclared variables.
     ╭─[src/helpers.js:172:38]
 171 │     if (window.debugging === "all" || window.debugging?.includes("machines"))
 172 │         console.debug.apply(console, arguments);
     ·                                      ─────────
 173 │ }
     ╰────
  help: 'arguments' is not defined.

That's not in the GLOBALS in eslint so I am not sure how they detect that.

It seems like the scopes for functions always contains the arguments.

https://github.com/eslint/eslint/blob/7bcda760369c44d0f1131fccaaf1ccfed5af85f1/tests/lib/languages/js/source-code/source-code.js#L2271

@jelly
Copy link
Contributor Author

jelly commented Jul 11, 2024

All that's left now seems to be:

[jelle@t14s][~/projects/cockpit-machines]%~/projects/oxc/target/debug/oxlint --config .eslintrc.json src

  × eslint(no-undef): Disallow the use of undeclared variables.
     ╭─[src/helpers.js:172:38]
 171 │     if (window.debugging === "all" || window.debugging?.includes("machines"))
 172 │         console.debug.apply(console, arguments);
     ·                                      ─────────
 173 │ }
     ╰────
  help: 'arguments' is not defined.

That's not in the GLOBALS in eslint so I am not sure how they detect that.

It seems like the scopes for functions always contains the arguments.

https://github.com/eslint/eslint/blob/7bcda760369c44d0f1131fccaaf1ccfed5af85f1/tests/lib/languages/js/source-code/source-code.js#L2271

Yup, that should be the case, something to fix as follow up :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category - Bug
Projects
None yet
3 participants