SublimeLinter / SublimeLinter-eslint

This linter plugin for SublimeLinter provides an interface to ESLint
MIT License
865 stars 88 forks source link

ESlint does not find any errors. #300

Closed faaizajaz closed 2 years ago

faaizajaz commented 4 years ago

Sorry if this has come up before, but I sifted through closed issues and did not find anything that quite matches.

I've installed the latest (at this time) version of SL and the ESlint plugin from Package Control on ST 3.2.2, Build 3211.

When I run ESlint from the command panel, I get the following output in the console with debug=True:

SublimeLinter: sublime_linter.py:345  Delay linting 'index.vue' for 0.0s   
SublimeLinter: linter.py:1088         eslint: Checking lint mode 'background' vs lint reason 'on_load'.  Ok.
SublimeLinter: SublimeLinter/SublimeLinter#1 linter.py:1105      eslint: linting 'index.vue'
SublimeLinter: SublimeLinter/SublimeLinter#1 node_linter.py:85   Searching executable for 'eslint' starting at '/home/faaiz/documents/websites/foo/pages'.
SublimeLinter: sublime_linter.py:593  Linting 'index.vue' took 0.00s
SublimeLinter: linter.py:1088         eslint: Checking lint mode 'background' vs lint reason 'on_user_request'.  Ok.
SublimeLinter: sublime_linter.py:345  Delay linting 'index.vue' for 0.0s
SublimeLinter: linter.py:1088         eslint: Checking lint mode 'background' vs lint reason 'on_user_request'.  Ok.
SublimeLinter: SublimeLinter/SublimeLinter#2 linter.py:1105      eslint: linting 'index.vue'
SublimeLinter: SublimeLinter/SublimeLinter#2 node_linter.py:85   Searching executable for 'eslint' starting at '/home/faaiz/documents/websites/foo/pages'.
SublimeLinter: sublime_linter.py:593  Linting 'index.vue' took 0.07s

In this case, it seems like the linter is working, however I do not see any errors either in the console or in the editor. Through the console, I can verify that background linting through SL is working as expected.

Curiously, it seems to be searching for the eslint executable within the folder of the current file open in ST. So in this case, /home/faaiz/documents/websites/foo/ is the project root, and it is looking in /home/faaiz/documents/websites/foo/pages/. The file being linted is /home/faaiz/documents/websites/foo/pages/index.vue.

No errors of any sort. ESlint works fine from the command line.

kaste commented 4 years ago

Horrible unhelpful log. What it does is it actually does not lint but aborts. What I think what happens is: Here you're having a vue file, eslint generally supports vue so it goes on, but then it thinks you don't have the eslint vue plugin installed and aborts silently. Idk if you have the plugin installed. It might be correct about that. In any case the log is 😭.

faaizajaz commented 4 years ago

Thanks, it was difficult for me to understand what is going on from the logs.

I do have ESlint installed--it is in my packages.json and also installed globally using npm install -g. Is it possible that there is a conflict between the two installs? As I said, running eslint from the CLI works as expected. So I don't think it's a path issue?

For what it's worth, the other ESlint ST plugin works fine, but obviously I greatly prefer SL for the real time linting.

kaste commented 4 years ago

Since the logging is :shit: here, there is more guessing and trying. You can open a sibling normal "js" file; this should work, and from the debug log you should see which executable we choose here, the local one or the global one.

For global installations, there is no automatic "vue" support at all. (You can configure it manually, we come back to that when necessary.) Local installations should have a eslint-plugin-vue entry in package.json either under dependencies or devDependencies. (In the same package.json where you also defined eslint as a dependency itself.) If everything is correct here, it still could be that package.json is not valid (simple) json (for example has comments, dangling commas) in which case we cannot read it and also abort silently.

kaste commented 4 years ago

What we should address here is to proper logging of all silent exit points in https://github.com/SublimeLinter/SublimeLinter-eslint/blob/0983eba526d644d9293fb0e8943e60c3afe235dc/linter.py#L62-L98

That is whenever we fall through and then raise PermanentError we should have logged why.

rgant commented 3 years ago

At least for me it seems that Sublime/Linter cached the previous plugins/package.json and didn't update when I installed the plugin. I tried restarting Sublime and that didn't help, but uninstalling and re-installing SublimeLinter-eslint did.

kaste commented 3 years ago

@rgant We do cache but only in-memory and also check if the files has been touched iirc so reinstalling solved maybe a different setup issue.

rgant commented 3 years ago

You are correct, it doesn't appear to be resolved with uninstalling and reinstalling. I did unzip the package to add prints to debug and while that didn't even get me to the part of the code where it bailed out yet, something about that fixed things until I was once again re-installing things.

Now I see that self.context.get('project_root') is returning None which is causing the bail-out.

rgant commented 3 years ago

Best I can tell the problem is that node_linter.py (in the super project) doesn't always set project_root.

If this if is True: https://github.com/SublimeLinter/SublimeLinter/blob/35a7afe930d9fc8d3e5acc523bfef8a4f21a8256/lint/base_linter/node_linter.py#L77

Then find_local_executable is never called, which is the only place I see in either project setting project_root: https://github.com/SublimeLinter/SublimeLinter/blob/35a7afe930d9fc8d3e5acc523bfef8a4f21a8256/lint/base_linter/node_linter.py#L119

It's hard for me to say if maybe Sublime has a context of "project_root" that it sets somewhere, but I don't see anything for that in my searching.

rgant commented 3 years ago

My local hack fix is:

project_root = self.context.get('project_root') or self.context.get('project_path')

If that seems reasonable I can open a pull request to add that.

kaste commented 3 years ago

It bails out at SublimeLinter/SublimeLinter@35a7afe/lint/base_linter/node_linter.py#L77 if you configured executable manually because we cannot find the "package.json" from the executable, only the other way around. As you already bailed out of the "automatic" (by setting executable), it seems reasonable to set selector manually as well I think.

Maybe you can avoid setting executable so we run find_local_executable, or we find out why find_local_executable fails and if we can improve its algo.

rgant commented 3 years ago

Understood. I set the executable because I don't want this project to accidentally use a globally installed eslint, it should fail ugly if someone on my team fails to run npm ci. At least that is my intention.

Still seems reasonable to me that 'project_path' is likely to be where package.json and node_modules is. Much better than just silently not linting a file. Although I suppose I can just also specify the selector.

kaste commented 3 years ago

just silently not linting a file

The scope here is just automatically detecting special plugins and syntaxes. It will lint "js" files. I don't like the "silently" part though.

I don't want this project to [...] use a globally installed eslint

We have the setting disable_if_not_dependency for that.

It somewhat sound reasonable, but if we provide a fallback, we should use folder.

project_root = self.context.get('project_root') or self.context.get('folder')
IPWright83 commented 3 years ago

I think I'm getting the same issue, read through the notes but I'm not 100% sure what the fix I need to make is?

SublimeLinter: linter.py:1127         eslint: Checking lint mode 'background' vs lint reason 'on_save'.  Ok.
SublimeLinter: #8 linter.py:1144      eslint: linting 'OccupancyHeader.test.tsx'
SublimeLinter: #8 node_linter.py:83   Searching executable for 'eslint' starting at '/home/ian/src/........./OccupancyHeader'.
SublimeLinter: sublime_linter.py:593  Linting 'OccupancyHeader.test.tsx' took 0.04s

But running via a CLI

/home/ian/src/webclient/app/src/............/OccupancyHeader/OccupancyHeader.test.tsx
   1:1   error    Definition for rule 'testing-library/no-node-access testing-library/no-container' was not found      testing-library/no-node-access testing-library/no-container
   4:1   warning  There should be at least one empty line between import groups                                        import/order
   5:1   warning  `@testing-library/user-event` import should occur before import of `testUtils`                       import/order
  15:26  warning  Avoid direct Node access. Prefer using the methods from Testing Library                              testing-library/no-node-access
  25:1   error    This line has a length of 274. Maximum allowed is 120                                                max-len
kaste commented 3 years ago

I think you can try to set selector manually. You can also hack on the plugin and log where we abort and turn that into a PR (see: https://github.com/SublimeLinter/SublimeLinter-eslint/issues/300#issuecomment-704506799).

IPWright83 commented 3 years ago

Thank you @kaste.

For anyone else my initial reaction was "What is a selector?". https://github.com/SublimeLinter/SublimeLinter-eslint#examples has some example selectors. I've a mix of all sorts so went with: source.tsx, source.ts, source.jsx, source.js - meta.attribute-with-value