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

Unused eslint directives are not detected #12186

Open
javagl opened this issue Sep 7, 2024 · 2 comments
Open

Unused eslint directives are not detected #12186

javagl opened this issue Sep 7, 2024 · 2 comments

Comments

@javagl
Copy link
Contributor

javagl commented Sep 7, 2024

There are a few eslint-disable... directives that are unused. They are detected by VSCode, but apparently, don't cause any problem in CI:

Cesium Unused Eslint

One way to check for them is to change the eslint call to include --report-unused-disable-directives, as in

"eslint": "eslint \"./**/*.*js\" \"./**/*.html\" --cache --quiet --report-unused-disable-directives",

But preferably, this should be included somewhere in https://github.com/CesiumGS/eslint-config-cesium .

The documentation mentions this:

To report unused eslint-disable comments, use the reportUnusedDisableDirectives setting. For example:

export default [
    {
        linterOptions: {
            reportUnusedDisableDirectives: "error"
        }
    }
];

So it might be trivial to add this for someone who knows the right place.


Running eslint with --report-unused-disable-directives currently generates 149 messages 😬 But it looks like they can all be fixed automatically with --fix. So this could easily be done as a one-shot cleanup.

(Some of them are about to be fixed as part of another PR, so it could make sense for this PR to be merged before doing that cleanup pass)

@Tim-Quattrochi
Copy link

Tim-Quattrochi commented Sep 26, 2024

@javagl I can tackle this after the PR you referenced is finished. I took out the ones in MetadataTableProperty.js in said PR since you mentioned the boy scout rule.

@javagl
Copy link
Contributor Author

javagl commented Sep 26, 2024

This issue mainly referred to the build process and configuration. (I.e. it was not about the eslint warnings themself, but about the fact that they are not emitted during the build).

I think that some setting has to be changed in https://github.com/CesiumGS/eslint-config-cesium (but am not sure where and how).

But of course, after that is enabled, the warnings that it will cause here, in CesiumJS, will have to be fixed. (That should be a largely automatic process, but still has to be done in a dedicated PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants