Remove undefined checks

[jprichardson]

This breaks on global variables, specifically mocha tests (it and describe). As mentioned here 91e8a41 your jshint checks are overzealous and defeat the purpose of this module, which states that it's for style checking.

[feross]

This is why mocha's implicit globals are a bad idea! ;) But jokes aside, thanks for pointing out that this is an issue. I'd like JavaScript Standard Style to work out-of-the-box even when people are using mocha, and would like it to work for you!

So, a couple of options to solve this:

• Add it and describe to the globals section of .jshintrc. This solves the immediate issue.
• Remove the check for undefined variables. I'm less excited about this solution because catching undefined variables actually helps find bugs, dead code, and typos. For example, it uncovered this dead code in bittorrent-tracker (socket is undefined) which would have been a runtime error if it weren't wrapped in a try-catch by chance.

I'm curious why you think reporting unused variables isn't a style issue - could you elaborate?

[feross]

Here's another example where this option saves the day:

function test () {
  var myVar = 'Hello, World'
  console.log(myvar) // Oops, typo here - would be a runtime error, but caught by `standard`
}

[feross]

Actually, that last example would be caught by the undef option, even if we disabled the unused option.

undef: prevent using a variable that was never defined. Ex: myvar is used, but was never undefined.
unused: prevent defined a variable and never using it. Ex: myVar is defined, but was never used.

[feross]

Okay, I remove the unused rule, but kept the undef rule in place.

Defining a variable and never using it, while confusing, isn’t going to cause break at runtime like the reverse case (using an variable that was never defined) will.

Cheers!

[dcousens]

Relaxing the undef rule opens up to a lot of bugs which I am/was hoping to catch using this standard in my tests (I already run jshint to catch all these locally).
So I'm happy that is still around.

Personally, I actually feel unused could have stayed, if your local tests are tied directly to standard, unlink them or add a convenient shortcut.

In pbkdf2-compat I use two different test targets to avoid this situation.
Granted, that is my personal opinion.

Adding the mocha globals might have been all that was necessary IMHO.

[feross]

Okay, after an in-depth discussion on IRC with folks, we've decided to:

• Re-add the rule to disallow unused variables. [https://github.com/feross/standard/commit/8134d23c8fab6d05acc2328bf4108caf2bd5e7e5]

It turns out that this rule actually does catch pretty serious errors. Case in point, it just caught this craziness in one of my own modules. We couldn't come up with a legitimate reason why an unused variable should ever stick around.

• Remove the exceptions for mocha's describe and it globals. [https://github.com/feross/standard/commit/7695e2b4f656c082d1f3074d110dcf59506b2de4]

If you use mocha, you can explicitly allow describe and it with a /* global describe, it */ comment at top of each test file. This is a better solution than assuming these globals are always defined across all files. Also, mocha actually defines a bunch of other globals like beforeEach, etc. and this would have been a slippery slope.

[feross]

These changes were released as 2.0.0, along with other improvements like adding ESLint.

[jprichardson]

undef: prevent using a variable that was never defined. Ex: myvar is used, but was never undefined.
unused: prevent defined a variable and never using it. Ex: myVar is defined, but was never used.

You won't find any disagreement from me whether these can help improve our code. My position was centered around two things:

1. This module presented itself to be more concerned with style than semantics. As far as I can tell, this was the stated intention. One can infer it from the statement: "one standard style to rule them all".
2. It's much easier to add things than to remove them. i.e. Being overzealous in the beginning may have disastrous consequences in the long run. As we may find that some of these choices may really hinder our development. I'm more in favor of limited decisions and having the rest come organically as they make sense.

That being said, I wasn't aware that I could declare globals at the top of the file via a comment. I actually really like that. Also, I didn't consider that I can just have a npm test with standard && mocha and just run mocha while doing actual development. When I'm ready to commit, I can verify that everything works by running npm test before I commit.

All in all, I'm happy to embrace it and am excited to just tell people, committers, etc to "just use standard JavaScript" :)

[feross]

Sweet, glad that you agree with the changes!

It's much easier to add things than to remove them

Actually, I think the reverse is true. If we add a rule, we have to bump the major version because we might cause someone's build to suddenly start failing.

If you add standard to a new or existing project and find that one of the rules is super tedious, we can easily just remove it. That would only be a minor version bump.