Skip to content
This repository has been archived by the owner on Oct 19, 2021. It is now read-only.

Create ScopeLinter #415

Closed
AsaAyers opened this issue May 10, 2015 · 17 comments
Closed

Create ScopeLinter #415

AsaAyers opened this issue May 10, 2015 · 17 comments

Comments

@AsaAyers
Copy link
Collaborator

When I created coffeelint-undefined-variables due to poor design, I ended up mixing the "undefined variables" and "unused variables" rules together. A few weeks ago I started over on that idea and tonight I have published the first bits of code.

I created a new project (coffeescope) that scans a file and produces nested scopes listing all variables and references. With that information available I'm now creating a ScopeLinter so we can have different scope related rules.

Right now the CoffeeLint branch scope-linter is just a stub. There are no rules yet, but i added the linter without breaking tests.

Once I get this all done and have the undefined_variables and unused_variables rules implemented I'll deprecate coffeelint-undefined-variables

@katspaugh
Copy link

Wonderful news!

@hccampos
Copy link

This is awesome news! It is one of the biggest things that was missing in Coffeelint.

@trshafer
Copy link

Looking forward to this! Thanks!

@AsaAyers
Copy link
Collaborator Author

The scope-linter branch just got two new rules no_undefined_vars and no_unused_vars. If anyone is interested in helping out you can run the branch and contribute over at CoffeeScope.

Here are some of the types of errors I'll be working on next, as they show up when running coffeelint against itself:

False Positives

CoffeeScope isn't finding where these are defined even though they are:

  ✗ src/rules/ensure_comprehensions.coffee
     ⚡ #18: Unused variable. idents.
     ✗ #24: Undefined variable. prevToken.

CoffeeScope isn't finding where these are used even though they are:

  ⚡ src/rules/no_tabs.coffee
     ⚡ #18: Unused variable. indentation.

Crash

CoffeeScope simply can't scan some of the files. It wont' crash CoffeeLint but none of the scope rules will run against the file when it can't be scanned.

  ⚡ src/ast_linter.coffee
     ⚡ #1: CoffeeScope Error: Cannot call method 'unwrapAll' of null.

@mitar
Copy link
Contributor

mitar commented May 19, 2015

Just a comment here: in our code style we prefer having all function/callback arguments always written fully out, even if not used. But unused variables declared in function scope should be detected. So consider adding a switch which would allow unused function arguments.

@AsaAyers
Copy link
Collaborator Author

yup, got it. I even works just like http://eslint.org/docs/rules/no-unused-vars

I actually have the rules written but I haven't merged it because CoffeeScope still misses variables and references all over the place. It gets a bunch of false positives just trying to lint coffeelint's source right now.

Does anyone here have a large coffeescript codebase I could try these rules out on when they're done? I don't know a better way to make sure it works than to just run it against as much code as I can get my hands on. I've been adding samples of false positives to the test suite so I can know when they're all fixed.

@mitar
Copy link
Contributor

mitar commented May 19, 2015

@katspaugh
Copy link

@trshafer
Copy link

@mitar, that's a great distinction. We like to be explicit in all of the function parameters as well.

@giladgray
Copy link

👍 this is awesome, was just wondering if such a thing exists. great minds think at the same time.

@brunocoelho
Copy link

👍

@AsaAyers
Copy link
Collaborator Author

AsaAyers commented Jul 1, 2015

From CoffeeScope:

CoffeeScript's AST doesn't have all of the information needed to determine how variables are used. The simplest example is implicit returns, they are simply missing from the AST. My workaround was to have the AST compile itself because for some dumbass reason compiling the AST allows it to modify itself and it puts in the explicit return. This introduces new problems; now the AST contains code you didn't write. Your default parameter now exists as part of the parameter AND inside an if statement in the body. This is a major problem when trying to lint what the user wrote.

I expect I am probably going to abandon this project.

If anyone wants to pick up the project and continue it you're welcome to, but my plan is to just move to using ES6 /w Babel for all new code. CoffeeScript was good for a while, but now there's no need for it.

@AsaAyers AsaAyers closed this as completed Jul 1, 2015
@mitar
Copy link
Contributor

mitar commented Jul 1, 2015

Which project are we talking about here now? CoffeeScope or CoffeeLint?

@AsaAyers
Copy link
Collaborator Author

AsaAyers commented Jul 2, 2015

Mostly coffeescope. Without a complete list of variable declarations and references per scope you can't actually locate undefined and unused variables.

I don't really see any other big things that coffeelint needs. I'll continue to be here to manage it.

@mitar
Copy link
Contributor

mitar commented Jul 7, 2015

I don't really see any other big things that coffeelint needs. I'll continue to be here to manage it.

Great! :-)

@mwager
Copy link

mwager commented Oct 7, 2015

sad :( is there really no tool for doing unused variable detection in coffeescript? the only thing I know is webstorm..

@za-creature
Copy link
Contributor

I've made some efforts towards improving the situation: https://github.com/za-creature/coffeescope

While I've started using it successfully for my own code, there are many subtleties to coffeescript that the project in its current form is probably missing. Please try it out and report any bugs or missing features.

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

No branches or pull requests

9 participants