standard / standard Goto Github PK

Standard will fail when given ES6 destructuring assignments:

Is it supposed to do this?

to make it as easy as possible to adhere to this style for the devs

This might have something to do with the installed toolset on my machine, but i tried running standard after installing globally (npm install -g standard), and I am getting an error thrown on what appears to the standard's index file.

➜  lib (master) standard
Error: Code style check failed:

/usr/local/lib/node_modules/standard/index.js:90
              var fileA = FILE_RE.exec(a)[1]
                                         ^
TypeError: Cannot read property '1' of null
    at /usr/local/lib/node_modules/standard/index.js:90:42
    at Array.sort (native)
    at /usr/local/lib/node_modules/standard/index.js:88:14
    at ChildProcess.<anonymous> (/usr/local/lib/node_modules/standard/index.js:133:21)
    at ChildProcess.emit (events.js:98:17)
    at maybeClose (child_process.js:756:16)
    at Socket.<anonymous> (child_process.js:969:11)
    at Socket.emit (events.js:95:17)
    at Pipe.close (net.js:465:12)

Have you seen this before, and/or know what the issue might be?

mkdir /tmp/new
cd /tmp/new
standard

if you don't have any js files, in /tmp, it should just hang forever.

Using the new ES6 syntax, I have several template strings passed as arguments to a function or method, formatted like this:

dothing(`url${variable}`)

Standard reports an error at the end of the line. The error text is:

Illegal space before closing round bracket

Missing space before function parentheses.
Example:
https://gist.github.com/anonymous/239cfd747eca6072c53e

Fails on "set variable (value) {}"

When I use istanbul in a project, it generates some js files in the coverage directory. This directory is in the Github-default .gitignore, and standard should probably ignore it by default too.

Since your using three linters your tool will report errors multiple times.

We build a deduplicator into lint-trap cc @malandrew

• https://github.com/uber/lint-trap/blob/master/dupe-map.json
• https://github.com/uber/lint-trap/blob/master/dedupe-transform.js

You may want something similar :D #goodluck

this is a fairly common practice. some reasoning can be seen here: http://stackoverflow.com/questions/2287967/why-is-it-recommended-to-have-empty-line-in-the-end-of-file
here's 'proof' it is counted as an error:

I like standard; having gone through the exercise of converting what has to be one of the largest, most complicated code bases to support it so far, I find myself happy (or at least satisfied) with every choice it makes except the fact that it prescribes both the browser and node modes in eslint (for reasons I can get into in another issue, if desired).

But! I'm doing a lot more work with ES6 in my code now, and it would be nice to just go ahead and enable ES6 features as they roll into eslint (and jscs). It's probably going to be a little while before eslint supports everything (they just landed support for destructuring), but right now I have to keep switching between standard and eslint in my Syntastic configuration, and that feels suboptimal.

People say it's a bad idea, i.e. hard to get right. I thought that browserify was making it work somehow, so thought we could make it work too, but @substack says it's not.

Let's just make the user explicitly pass - if they want stdin, like browserify does. This will probably fix the vim and syntastic issues we've been seeing: #65, #64

bauchelain% standard --version
3.0.0-beta
bauchelain% standard
Error: Use JavaScript Standard Style (https://github.com/feross/standard)
  src/models/album-base.js:2:15: Missing space before function parentheses.
  src/models/album-multi.js:8:15: Missing space before function parentheses.
  src/models/album-multi.js:9:4: 'super' is not defined.
  src/models/album-multi.js:16:11: Missing space before function parentheses.
  src/models/album-multi.js:21:10: Missing space before function parentheses.
  src/models/album-multi.js:32:8: Missing space before function parentheses.
  src/models/album-multi.js:46:11: Missing space before function parentheses.
  src/models/album-single.js:4:15: Missing space before function parentheses.
  src/models/album-single.js:5:4: 'super' is not defined.
  src/models/album-single.js:15:11: Missing space before function parentheses.
  src/models/artist.js:2:15: Missing space before function parentheses.
  src/models/artist.js:8:18: Missing space before function parentheses.
  src/models/artist.js:12:11: Missing space before function parentheses.
  src/models/cover.js:4:15: Missing space before function parentheses.
  src/models/cover.js:12:11: Missing space before function parentheses.
  src/models/track.js:4:15: Missing space before function parentheses.
  src/models/track.js:20:11: Missing space before function parentheses.
  src/models/track.js:24:12: Missing space before function parentheses.
  src/models/track.js:32:12: Missing space before function parentheses.

As you can see, these are pretty standard uses of class syntax, and transpile without issue with babel. I haven't had the opportunity to dig in deeper and figure out what espree rules are causing these issues.

Am using the standard command line tool from this project and am working on a Chrome project. Does that mean that I still need to use package.json and add the standard.ignore array? Or is there any generic way to ignore files?

For a couple of reasons:

1. self resolves to the global window context in the same way that this does, well at least that's been my experience and have been confused by it in the past. Open the dev console in chrome and eval self for an example.
2. It assumes that there is only one item that will have it's context dissociated on callback firing. While this is generally true when you are writing nice clean code, there are occasionally instances where this will not be true.

Like I said though, this is just my opinion...

Thanks for this.

I'm using it as such:

{
  "name": "foo",
  "devDependencies": {
    "watch": "^0.13.0",
    "standard": "*",
    "jasmine": "^2.2.0"
  },
  "scripts": {
    "watch": "watch 'npm test'",
    "test": "standard && jasmine"
}

When I run $ npm test all is well and as expected.

The issue comes when I use watch, such as $ ./node_modules/.bin/watch 'npm test' or $ npm run watch.

The output from standard is not printed. I know that standard runs because jasmine's output is printed.

Do you think that watch is doing something wrong?

So, I'm trying to use standard on .jsx files. And I run into an issue where I can't use //jscs:disable and /* eslint-disable */, because they still depend on the AST parser and will throw an error if they can't parse the JSX templating.

Example file:

, render: function() {
    //jscs:disable
    /*eslint-disable */
    return <div className="form-horizontal">
      {this.renderTextInput('subject', 'Subject')}
      {this.props.email && this.renderTextInput('email', 'Email')}
      {this.renderTextInput('message', 'message')}
    </div>
  //jscs:enable
  /*eslint-enable */
  }

What do you think about this? I could potentially use fs.readFileSync(__dirname, '/form-template.jsx') and pipe the rendering jsx template through that, but that seems messy. Do you think there would be a way of using standard on these files, otherwise?

the global variable in a web worker is named self, so renaming this to self in a worker can be tricky if you then need to grab a reference to the global, does this assume it's always in a browserify context?

I really like this forced standard! It's simple and will be effective. The only thing I can't get my head around is the 2 space indentation.

Why not use tabs instead? So that everyone in the team can configure their own horizontal depth?

My best case against conflating the two modes is this incredibly silly change I had to make to npm to get the affected files through standard. I know that window.opener is a thing in browsers, but it has not been and never will be part of node, and this error message (like the one you get for crypto in some contexts) makes no sense unless you know / care about the browser-side problem it's trying to address.

I tried very hard to figure out how to remove / override the relevant global check via code comments, but either I don't understand how eslint applies those overrides, or it's simply not supported. Either way, I get that a lot of code will be browserified at some point and thus should pass the browser checks, but there is no plausible or reasonable reason to run npm in the browser (phrased that way because I know somebody probably is going to try anyway at some point). It should be OK for server-side apps to say "I am a server-only thing".

nodejs/node-v0.x-archive#2318 strikes again.

This workaround fixes it for me:

if (/^win/.test(process.platform)) {
  JSHINT += '.cmd'
  JSCS += '.cmd'
}

I find I have to double take when I look at this, to make sure I'm not mis-understanding what is actually part of the expressions that get executed.

The example is the following:

if (condition)
  expression
else
  more expressions
otherstuff

I think that is confusing and unnecessary, and IMHO it should only either be:

if (condition) expression
else more expressions

or

if (condition) {
  expression
} else {
  more expressions
}

edit: Infact, I only ever use the shorthand one-liner for early-exit return statements, but YMMV.

@feross thoughts?

To reproduce, create a ~/.eslintrc file with this in it:

{
  "rules": {
    "no-var": 2
  }
}

Next, checkout the latest copy of standard and run npm test. You'll see many errors and the test will fail. This seems to be a feature of eslint: http://eslint.org/docs/configuring/#configuration-cascading-and-hierarchy

Note that this only affects rules that are not yet defined in rc/.eslintrc due to the layering/cascading nature. I see two possible solutions:

• define every rule in rc/.eslintrc to stop the possibility of cascading
• figure out a way to explicitly turn off the cascading feature when calling eslint.

Maybe @nzakas knows if there is a flag to stop this behavior? I did not see one in the command line options or on the linked configuration page.

The only workaround for users would be to remove any .eslintrc files from their directory hierarchy, which is probably not acceptable.

Running within vim completely destroy the Vim window. You can run it by issuing (:! standard) in vim.

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.

Which makes me sad because now it seems like most of the ES6 stuff I use is handled properly. ;_;

I updated Syntactic to HEAD, and have verified that standard is working by running :!standard against an erroneous file from inside vim. If nothing obvious occurs to you about how the formatting might have changed in beta4, I can dig into this more tomorrow.

My VIM doesn't recognize that a project is Standard.

Is the presence of standard in package.json's devDependencies enough to flag that the package is Standard?

Perhaps both ESLint and JSCS should be patched to recognize the presence of Standard and then automatically look for the RC files in the right place?

Then, when used from within the editor, it just happens automagically—well, at least with some editors, who rely on external tools like that.

I use mocha, and standard complains about the undeclared variables (like describe, it, and before) that it uses. Of course, I can add

/*eslint-env mocha */

but that seems like something that could be handled by standard. Perhaps inspecting package.json to look at dependencies and directories.test, or just making some common guesses.

Perhaps this is outside of the scope, but it certainly would make me feel more standard if I could just arrange my projects in a specific way and ignore the linter directives entirely.

As I'm checking and porting my code, it's very convenient to do it piecemeal. Would you consider adding stdin support so that devs can do this:

standard < myfile.js

Like I said, it has the added bonus of making it much easier to port large codebases over. Thanks!

Run standard to recursively check a whole project.
Run standard < index.js to check an individual file.

Need to support: standard index.js or standard index.js test.js lib/a.js to check a list of files. Shouldn't be hard to add.

This is awesome.

Any thoughts on row width?

we use the src/node_modules pattern in our apps (e.g. here and here), so the default ignore of **/node_modules/** doesn't work for us.

previous discussion in #35.

possible solutions already brought up:

1. "we could ignore node_modules if a package.json is present in the same directory"
2. implement a way to un-ignore a directory

1 makes a lot of sense, i don't see how it wouldn't work as the src/node_modules pattern as i understand it doesn't involve a src/package.json. keen to hear what people think, am happy to implement a PR for this.

Explanation: http://andrewkelley.me/post/js-callback-organization.html

I've installed standard and tried to run it over my codebase, however it always seems to fail on this line:

/Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard/index.js:176
        var lineA = Number(LINE_RE.exec(a)[1])

I've tried installing standard locally to my project and as a global module, the issue is the same. When I had installed standard at version 2.3.1 in another project it didn't fail, so I imagine this issue has arisen between that version and the current one.

Below is a copy of my terminals output when installing and running standard on my project.

~/Projects/OfflineAudioOrg/OfflineAudio.github.io master*  11s
❯ npm i -g standard
/Users/jakechampion/.nvm/v0.10.28/bin/standard -> /Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard/bin/cmd.js
[email protected] /Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected]
├── [email protected] ([email protected])
├── [email protected] ([email protected], [email protected], [email protected])
├── [email protected] ([email protected])
├── [email protected] ([email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected])
└── [email protected] ([email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected])

~/Projects/OfflineAudioOrg/OfflineAudio.github.io master*  14s
❯ standard
Error: Code style check failed:

/Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard/index.js:176
        var lineA = Number(LINE_RE.exec(a)[1])
                                          ^
TypeError: Cannot read property '1' of null
    at /Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard/index.js:176:43
    at Array.sort (native)
    at printErrors (/Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard/index.js:171:8)
    at /Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard/index.js:128:28
    at done (/Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard/node_modules/run-parallel/index.js:15:13)
    at ChildProcess.<anonymous> (/Users/jakechampion/.nvm/v0.10.28/lib/node_modules/standard/index.js:136:7)
    at ChildProcess.EventEmitter.emit (events.js:98:17)
    at maybeClose (child_process.js:753:16)
    at Socket.<anonymous> (child_process.js:966:11)
    at Socket.EventEmitter.emit (events.js:95:17)

Using just a single linter would simplify standard, and even speed it up (#43).

eslint is excellent, and we've been slowly moving rules from jscs to eslint whenever possible. There are only a few rules that are unique to jscs at this point. The eslint project wants to support jscs options that make sense, so I think we'll be able to switch entirely very soon. These are the options we're waiting for:

• disallow multiple var declaration (eslint/eslint#1619)
• Force dot after line break (eslint/eslint#1884)
• require operator before line break (eslint/eslint#1405)
• require spaces in for statement (eslint/eslint#1671)

It's awesome that standard supports jsx now.

In addition to supporting React, babel supports generic JSX transforms, using a pragma like this:

/** @jsx mypackage.doSomething */

demo

Of course, when using them, standard is peppered with 'React' must be in scope when using JSX. Would it be possible to respect this pragma. It's not a Babel invention, React itself required it until recently.

Consider this program:

var a = 1
if (a) {
  console.log('do something')
}

var b = 3
console.log(b)

;(function () {
  console.log('do another thing')
})()

Runs fine, no standard errors either.

Now consider this program:

var a = 1
if (a) {
  console.log('do something')
}

;(function () {
  console.log('do another thing')
})()

Runs fine, but standard prints the unnecessary semicolon error. While standard is indeed correct that the program and IIFE is fine without the semicolon prefix, the rule seems to be a bit too tight in this context. By default, I always prefix my IIFE with a ; so that I don't have to think of the contextual situations when it's necessary and when it is not. Do others do this? Is this a rule that should be relaxed? I'm guessing it's an eslint change.

Either way, this feels pretty low-priority to the other standard issues, but I thought that I'd bring it up for discussion.

Loving standard btw :)

First of all, I like the style of standard and I use it in all projects I started lately.

Buuuut, it always takes several seconds to test a single file in my test directory even if it only requires small files. My test file is about 150 LOC, the source is in this case even smaller.

Maybe this is an issue not with standard it self but with one of it's dependencies ... this is really interrupting my workflow.

[email protected]
Intel Core i5, 4 Gigs RAM, Ubuntu 14.04

ex:

switch (typeof thing) {
  case 'function':
    return doSomething()
  case 'string':
    return doSomethingElse()
}

throw new TypeError('wtf did u just give me')

and i'd make those cases one-liners if i know none of the cases are going to be multiple lines

Continuing from #31, I don't think that a Vim plugin is the correct path to take.

We can leverage existing code to achieve more results if we aim to use the default behaviors of JSCS and ESLint.

JSCS can be directed to a configuration file through package.json.

So if we use the install npm script of standard to inject a property to direct JSCS to our config file, so that it ends up like the following

{
    "name": "some-package",
    "jscsConfig": "node_modules/standard/rc/.jscsrc"
}

Then JSCS will serve us, not matter if it is run through Vim or any other editor/tool, most likely.

Is this about JSCS correct, @mikesherov, please?
Would it prevent JSCS from processing other .jscsrc files in the tree?

ESLint seems halfway there, because it can be configured via package.json but it doesn't have an include directive.

Adding a property to the package.json seems reasonable to me, since our users will see that change and not miss it. And it will be documented and it is only a small change. We just have to figure out how well this works/will work with JSCS and ESLint.

This approach may even simplify Standard's code a little.

Sometimes I do actually want ==. I can't think of any usecases now but I find that on occasion I explicitly opt for ==.

I'm wondering if some of these style restrictions could be selectively 'disabled' for a particular case if a comment is present to explain why an ill-advised practice was used instead of best practice.

"use strict"
var items = getCurrentItems()
// style: my religion speaks of a prophecy where vile, dark beings
// enter this realm and do unspeakable things to its inhabitants should 
// the chosen one (me, lol) fail to protect the semicolon of the 7th line of
// the 7th file. Remove this semicolon at your peril.
items = items.map(function(n) { return n * 2});
console.log(items)

maybe.

Using [email protected] and [email protected]:

~/src/yw/finfin master*  7s
❯ npm t

> [email protected] test /Users/yoshuawuyts/src/yw/finfin
> standard && NODE_ENV=test node test | colortape

child_process.js:1132
    throw errnoException(err, 'spawn');
          ^
Error: spawn E2BIG
    at exports._errnoException (util.js:734:11)
    at ChildProcess.spawn (child_process.js:1132:11)
    at Object.exports.spawn (child_process.js:964:9)
    at spawn (/Users/yoshuawuyts/src/yw/finfin/node_modules/standard/index.js:
162:20)
    at /Users/yoshuawuyts/src/yw/finfin/node_modules/standard/node_modules/run
-parallel/index.js:32:7
    at Array.forEach (native)
    at module.exports (/Users/yoshuawuyts/src/yw/finfin/node_modules/standard/
node_modules/run-parallel/index.js:31:11)
    at /Users/yoshuawuyts/src/yw/finfin/node_modules/standard/index.js:136:7
    at done (/Users/yoshuawuyts/src/yw/finfin/node_modules/standard/node_modul
es/run-parallel/index.js:15:13)
    at /Users/yoshuawuyts/src/yw/finfin/node_modules/standard/index.js:157:7
npm ERR! Test failed.  See above for more details.

~/src/yw/finfin master*

Google doesn't seem to be familiar with these popping up in node. Maybe anyone's had this happen before? The project can be found here in case you want to replicate. Thanks!

its not clear why this module would be necessary, or why its not in the pre-req modules if it is necessary.

$ npm install standard
npm WARN prefer global [email protected] should be installed with -g
[email protected] node_modules/standard

$ ./node_modules/.bin/standard api.js 
Error: Use JavaScript Standard Style (https://github.com/feross/standard)

Unexpected Linter Output:
module.js:340
    throw err;
          ^
Error: Cannot find module 'eslint-plugin-react'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)

When #47 is settled, it may be prudent to add '**/*.jsx' to default files list.

Currently standard only includes '**/*.js':
https://github.com/feross/standard/blob/master/index.js#L80

Currently, the --format flag only works when pointing at a file.

It should be updated to take input from stdin and send a formatted copy to stdout and send any lint errors to stderr.

I'm trying to write some tests for standard-format and want to run standard against some strings inside the test. So far the CLI seems to be the only thing documented. It would be nice to have some documentation on how/what I can do once I require('standard'). I'll try to contribute some after I get my head wrapped around what is available and opening this issue in the meantime.

How does standard compare to https://github.com/rwaldron/idiomatic.js/
I've enjoyed using standard due to the cli. It's very easy to run before committing code.
idiomatic.js looks very good from skimming the website. Are they pretty much the same? yes/no

I am on Windows 7 (64-bit) and did npm install standard -g and tried doing it.

I get this error:

c:\wamp\www\qtsite\src\js (master)
λ standard
Error: spawn ENOENT
    at errnoException (child_process.js:988:11)
    at Process.ChildProcess._handle.onexit (child_process.js:779:34)

Instead of:

/*eslint-disable no-unused-vars*/
var should = require('should')
/*eslint-enable no-unused-vars*/

Having:

var should = require('should') // eslint-ignore no-unused-vars