Call to arms: Scope linter, take 2

[za-creature]

Hello!

Is there anyone alive around here?

If so, I just finished my own take on a scope linter plugin and while my preliminary results have been satisfactory so far (on both synthetic tests as well as actual code), I think the project still has a bit to go before being ready for wide scale deployment.

To this end, I would like to ask any interested parties to try it out, and report any bugs or feature requests. Installation | Configuring

@AsaAyers Yes, I originally started this just to spite you. That being said, I would love any help you can offer here, especially what false positives / negatives did you encounter in your implementation.
@swang The end game here is to merge this back into coffeelint itself when it's deemed production ready. I don't yet know the best way to handle that right now, but we can start that discussion after a more rigurous smoke test.

[AsaAyers]

I don't know how to transfer ownership of coffeescope on npm, but if you want to publish your new version starting at at least 1.0.0, you're welcome to take over the name. I'm not interested in testing it, but I do wish you luck if that's what you want to build.

With the availability of ES6 through Babel I just consider CoffeeScript to be a waste of time. It's actually pretty easy to just copy CoffeeScript code into a .js file and fix the syntax errors. It mostly involves restoring the missing {} and ()

[AsaAyers]

I don't remember the details, but from what I remember of my CoffeeScope, the main problem I ran into is that CoffeeScript is simply missing information.

Watch CODE GENIUS - Rise of the Transpilers by Jeremy Ashkenas. The most important part for this discussion is his slide on Cheating starts at 11:56

We like to cheat and make things easier for ourselves ... this is not koshure, it is introducing context sensitivity into the lexer process. If you took the actual class you would get an F doing this kind of thing. But it's just programming, it's just code. You can cheat if you want to cheat

I think my problem was the implicit returns. When you iterate over the AST there is nothing to represent implcit returns, so a hanging variable is just unused. To get proper tracking you need to inject a return in there somehow. You could try to duplicate the logic in CoffeeScript to discover the implicit return, but this is ugly and may break with updates. CoffeeScript has to inject the return somewhow, so I dug through their code and found it. As you tell CoffeeScript to compile the AST actually modifies itself as it walks the tree. The node representing the function finds its last statement and adds the implicit return. This seemed like a solution at first, but it turns out to introduce other problems.

Again, this example is from memory from 8 months ago. I know that in having CoffeeScript compile first it broke default parameters. As I recall based on reading the post-processed AST this is how that got transformed.

// AST before compiling
(foo = "default") ->
  foo

// AST after compiling
(foo = "default") ->
  if foo isnt null
    foo = "default"
  foo

It seems like I got an error somewhere, but it could be as simple as the fact that foo is used by the generated code, so it's impossible to have an unused variable with a default parameter.

If you want to get https://www.npmjs.com/package/coffeescope just email me and I'll work with you on getting your project there. I think it might involve adding you as an admin on npm and then having you publish a new version from your repository.

[za-creature]

I'm not that fussed about the name TBH; I'd rather merge this back into coffeelint (as well as all other plugins that are permissive enough for this). Thanks for offering and for the well wishes and apologies if you were offended by my message, that was not my intention; I was just attempting to create a constructive rivalry.

With the availability of ES6 through Babel I just consider CoffeeScript to be a waste of time.

You are not alone in that opinion. In fact, I think most people would agree with you. Personally, I am working with Babel to pay the bills and have had some rather disappointing results with it; in fact, given the varying amount of support (or lack of) for ES6 in tools, using babel has been in some cases downright annoying (testing and creating coverage reports has been especially painful).

As for the language itself, I feel that while it's definitely a step in the right direction, it has a legacy that forces it to be syntactically (and sometimes semantically) awkward:

var, let and const scoped variables
for .. in to iterate over properties and for .. of to iterate over values
'single', "double" and interpolated strings
function() {} to create functions that lose context and () => {} to create functions that maintain it

And a whole bunch of redundant brackets.

None of these are huge problems or make the language unusable in any way; they just make it awkward to work with and make the code more complicated than it needs to be. Not that coffeescript doesn't have its own set of problems (I believe the scoping rules are among the most frequently criticized aspects) but all things considered, I find working around coffeescript's quirks to be much easier than working around the quirks of javascript.

Truthfully, I think it might be time for Coffeescript 2.0 which targets ES6 and newer and supports the newly added functionality in a less hideous interface (WebAssembly is still pretty far away from allowing other languages in the browser).

[AsaAyers]

I wasn't offended at all. Just a minute ago I was thinking about "Yes, I originally started this just to spite you. " and wondering "did he really think it would upset me?" I think it's the right phrase in context but struck me as a bit odd ¯_(ツ)_/¯

As far as CoffeeScript 2.0, that was called CoffeeScriptRedux and it's dead. There's also no benefit in transpiling to ES6 as you have to then transpile that to make it work in browsers. If CoffeeScript changes its target, it should move up to ES5. Now it's in this weird world where it targets ES3 + Generators but didn't bother to include a loop syntax that can use them

[za-creature]

When you iterate over the AST there is nothing to represent implcit returns

I might be missing something here, but I'm not sure why would they be needed in this case at all: aren't foo, foo(), return foo and foo + 1 semantically equivalent with respect to foo? (as in they all attempt to read it once and then maybe do something with it).

I didn't go down the compile-the-AST route but yeah, from what you're describing it would be impossible to detect unused arguments on that particular version of the compiled AST as the default code contains both a read and a write.

As far as CoffeeScript 2.0, that was called CoffeeScriptRedux and it's dead. There's also no benefit in transpiling to ES6 as you have to then transpile that to make it work in browsers.

I know about CoffeeScriptRedux (and of the rumors of its demise, whether exaggerated or not), but I thought that was supposed to be a less hacky drop-in replacement. What I was suggesting with "coffeescript 2.0" was something that targets ES6-capable devices (i.e. something that would only be useful in 2+ years when browsers have caught up) so that it can use all the new features (generators, async/await, Symbols, Proxies, module system) with a hopefully more sensible and consistent syntax. (not unlike this comment I noticed on the sad state of generator support in Coffeescript)

[AsaAyers]

I don't have a realistic example, but take a look at this code.

foo = () ->
  returnedValue = "something"
  uelessValue = "something"
  uselessValue
  returnedValue

If you just examine the AST, there is no return. Nothing anywhere is ever reading returnedValue. The only way to know it's being returned is to process that function body and locate the last statement. CoffeeScript leaves this step until the middle of writing JavaScript instead of putting that info in the AST where it belongs. uselessValue is a legitimate unused variable.

This is using an old version of CoffeeScript and CoffeeLint, but http://asaayers.github.io/clfiddle/ is nice for examining the AST. Originally I had plans to have it consume gists, so that people who wanted to post a CoffeeLint bug could put the code in there and we could actually see whether the issue was reproducible. In theory I would have updated it with new versions of CoffeeLint so I could point back to the same gist to show the bug was fixed.

[za-creature]

I think there are multiple interpretations to your example; I would argue that both variables are in fact read as evidenced by the output code:

// Generated by CoffeeScript 1.10.0
(function() {
  var foo;

  foo = function() {
    var returnedValue, uelessValue;
    returnedValue = "something";
    uelessValue = "something";
    uselessValue;
    return returnedValue;
  };

}).call(this);

While the first one is functionally useless in this case, it is technically read and with a slight adjustment to the code, it becomes significant (in that it causes a ReferenceError):

"""use strict"""
foo = ->
  uselessValue
  true

ESlint doesn't consider this case an unused variable either (though it picks up and warns about the standalone expression itself via a separate rule):

const blahblah = "true";
blahblah

The fiddle thing is pretty cool; I currently just run coffee --nodes foo.bar (which is why this file made it into the repo for a short while), but that thing is definitely better. Now if only someone would maintain it :)

[AsaAyers]

There's really no debate to be had. uselessValue in the produced JavaScript doesn't provide any value. It should be removed.

ESlint actually does detect this case:

All of this is just a distraction from the fact that as far as the AST is concerned, there is no difference at all between uselessValue and returnedValue. The distinction only happens as a side effect of the compilation to JavaScript.

As for clfiddle, anyone can fork the repo. It's a 2 year old project where no one has ever opened an issue, PR, or forked the project.

[za-creature]

There's really no debate to be had

Of course there is. You can choose to not participate, but that does not in any way refute my previous arguments.

ESlint actually does detect this case:

No, it detects the typo. If you change uelessValue -> uselessValue it will stop complaining (the error on the uselessValue; line you're encountering is actually an undefined variable error)

All of this is just a distraction from the fact that as far as the AST is concerned, there is no difference at all between uselessValue and returnedValue

The only difference is context: the value of a Block is defined as the value of the last expression it contains. I agree that extending this to functions is made awkward by the existence of Return nodes and implicitly adding such a node for the last child is probably a better way to handle this (although that would be against the notion that the AST should only contain code that was actually written). I just don't think it's that big of a deal.

As for clfiddle, anyone can fork the repo.

I was trying to be subtle there :)

[za-creature]

Closing this due to lack of interest