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

linter: oxlint VSCode extension not show warn or error for rules like import/no-cycle and import/no-duplicates #7118

Open
pumano opened this issue Nov 4, 2024 · 8 comments
Labels
A-linter Area - Linter C-bug Category - Bug

Comments

@pumano
Copy link
Contributor

pumano commented Nov 4, 2024

What version of Oxlint are you using?

0.11.0

What command did you run?

oxlint -c oxlintrc.json

What does your .oxlint.json config file look like?

{
  "$schema": "./node_modules/oxlint/configuration_schema.json",
  "plugins": ["import", "unicorn"],
  "rules": {
    "import/no-commonjs": "error",
    "import/no-cycle": "error",
    "import/no-duplicates": "error",
    "import/no-self-import": "error"
  }
}

What happened?

Rule import/no-commonjs works perfectly, but import/no-cycle or import/no-duplicates not show any sign of error or warn. Maybe problem in multiple spans in rule diagnostic or maybe in incorrect in terms of vscode large help info that not correctly handled by VSCode extension?

P. S. all rules works well in oxlint console output

@pumano pumano added A-linter Area - Linter C-bug Category - Bug labels Nov 4, 2024
@nrayburn-tech
Copy link
Contributor

Could you share the diagnostics or a reproduction for this? I tried something to reproduce locally, but wasn’t able to reproduce although I’m not super familiar with these rules either.

To get the diagnostics in the VS Code extension, set the “oxc-vscode.trace.server” setting to “verbose”. You’ll have to do this in the settings JSON, not the UI. The UI modifies an incorrect setting (issue for that is #7136).
Once you change that to verbose. Reload VS Code. Open the relevant file. Check the “Oxc (trace)” output channel. Share the published diagnostics from that channel. Make sure not to include anything you believe to be sensitive. That channel can contain file paths and full file contents.

@pumano
Copy link
Contributor Author

pumano commented Nov 6, 2024

Today I have more time to debug traces:

for import/no-commonjs diagnostic comes properly from oxc:

[Trace - 18:42:50] Sending notification 'textDocument/didSave'.
Params: {
    "textDocument": {
        "uri": "file:///Users/dz/my-org/my-app/libs/cdk/src/lib/test.ts"
    }
}


[Trace - 18:42:50] Received notification 'textDocument/publishDiagnostics'.
Params: {
    "uri": "file:///Users/dz/my-org/my-app/libs/cdk/src/lib/test.ts",
    "diagnostics": [
        {
            "range": {
                "start": {
                    "line": 2,
                    "character": 8
                },
                "end": {
                    "line": 2,
                    "character": 11
                }
            },
            "severity": 1,
            "code": "eslint(no-console)",
            "codeDescription": {
                "href": "https://oxc.rs/docs/guide/usage/linter/rules/eslint/no-console"
            },
            "source": "oxc",
            "message": "Unexpected console statement.",
            "relatedInformation": [
                {
                    "location": {
                        "uri": "file:///Users/dz/my-org/my-app/libs/cdk/src/lib/test.ts",
                        "range": {
                            "start": {
                                "line": 2,
                                "character": 8
                            },
                            "end": {
                                "line": 2,
                                "character": 11
                            }
                        }
                    },
                    "message": ""
                }
            ]
        },
        {
            "range": {
                "start": {
                    "line": 0,
                    "character": 13
                },
                "end": {
                    "line": 0,
                    "character": 30
                }
            },
            "severity": 2,
            "code": "eslint-plugin-import(no-commonjs)",
            "codeDescription": {
                "href": "https://oxc.rs/docs/guide/usage/linter/rules/import/no-commonjs"
            },
            "source": "oxc",
            "message": "Expected import instead of require\nhelp: Do not use CommonJS `require` calls and `module.exports` or `exports.*`",
            "relatedInformation": [
                {
                    "location": {
                        "uri": "file:///Users/dz/my-org/my-app/libs/cdk/src/lib/test.ts",
                        "range": {
                            "start": {
                                "line": 0,
                                "character": 13
                            },
                            "end": {
                                "line": 0,
                                "character": 30
                            }
                        }
                    },
                    "message": ""
                }
            ]
        }
    ]
}

For import/no-duplicates diagnostic only comes from eslint, but not from oxc:

[Trace - 18:50:07] Sending request 'textDocument/codeAction - (202)'.
Params: {
    "textDocument": {
        "uri": "file:///Users/dz/my-org/my-app/libs/cdk/src/lib/test.ts"
    },
    "range": {
        "start": {
            "line": 1,
            "character": 34
        },
        "end": {
            "line": 1,
            "character": 60
        }
    },
    "context": {
        "diagnostics": [
            {
                "range": {
                    "start": {
                        "line": 1,
                        "character": 34
                    },
                    "end": {
                        "line": 1,
                        "character": 60
                    }
                },
                "message": "'./table/table-navigation' imported multiple times.",
                "code": "import/no-duplicates",
                "codeDescription": {
                    "href": "https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-duplicates.md"
                },
                "severity": 1,
                "source": "eslint"
            }
        ],
        "only": [
            "quickfix"
        ],
        "triggerKind": 1
    }
}

after that I turn off import/no-duplicates in eslint and just retest it, just diagnostic not comes from language server.

@nrayburn-tech

how to test it:

  1. create file test.ts with 2 functions:
export function a() {
}
export function b() {
}
  1. import it from another file like:
import { a } from './test.ts';
import { b } from './test.ts';

@pumano
Copy link
Contributor Author

pumano commented Nov 6, 2024

for import/no-cycle same scenario: no diagnostics from server

@nrayburn-tech
Copy link
Contributor

I was able to identify where the problem is, but I don't know what the resolution is. Somebody else may be able to review and fix this, my findings are below.

The ModuleRecord.resolved_absolute_path value is missing when executing through the language server. However, when executing through the CLI it is available.

Here is where the import/no-duplicates rule uses it, but it looks like a few of the other import/* rules also use this value and are likely broken as well.

|module| module.resolved_absolute_path.to_string_lossy().to_string(),

@Boshen
Copy link
Member

Boshen commented Nov 7, 2024

@nrayburn-tech you are correct, the cli takes the cwd and all the paths

LintServiceOptions::new(cwd, paths).with_cross_module(builder.plugins().has_import());

Turning on the import plugin will hog memory and cpu as it needs to parse and store all ASTs and files. I'm not sure whether we should allow it.

If we allow it, it must be project opt-in. And we also need to rearchitect the language server to make it read the whole workspace.

@nrayburn-tech
Copy link
Contributor

nrayburn-tech commented Nov 7, 2024

I don't have any long term suggestions, but seeing different results between the CLI and the IDE is not a good user experience. It might be worthwhile to provide some sort of communication to the user when they are using an IDE plugin and have any import rules enabled.

The IDE plugins could possibly

  • highlight the import rules within the Oxlint config informing the user that the rule isn't supported
  • show a notification when parsing the Oxlint config and finding an import plugin rule (this would need to come with the option to disable the notification for a given project as well, so that users don't continue seeing it after acknowledging it once)

@pumano
Copy link
Contributor Author

pumano commented Nov 7, 2024

From my point of view: it's not a problem for oxlint run this rule every time in IDE, that rule very helpful for identifying circular imports and original rule import/no-cycle works with IDE, but we don't use them because in ci pipeline it gives us +10 mins of linting, but in IDE works well. For my project oxlint run less than in second, looks like not a problem and turning that will be helpful. Also import plugin not used by default, which not alter usual performance.

@pumano
Copy link
Contributor Author

pumano commented Nov 7, 2024

@Boshen what do you think about it? Maybe we can add flag to vscode config for turning that on if you think it harm to performance (which is not in my opinion) but that helps a lot for identifying circular deps in IDE.

import/no-duplicates looks looks like should be plain static analysis not a module visitor but not working too. Also oxc/no-barrel-file not working too which is helpful too.

I prepare my project to isolatedModules (and then in future isolatedDeclarations), but some rules just not working in IDE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter C-bug Category - Bug
Projects
None yet
Development

No branches or pull requests

3 participants