Ability to autoconfigure JSCS from a sample file

[azproduction]

I think .json config file is for robots. And we write this file because robots can't understand our code style from example. But they can do that and should.

Imagine that you have a reference .codestyle.js file, written by a human for humans, and a robot can understand your code style and apply it for the reset of your files.

var CONST_VALUE = 42;
/**
 * @param {Object} a
 */
function Constructor(a) {
    a = {
        a  : 1,
        ab : 2,
        abc: 3
    };

    var b = [ 1, 2, 3, [4] ];

    this['value'] = 123;
    var self = this;

    setTimeout(function () {
        self.value++;
    }, 10);
}

This file could contain all common code-style cases.

[gustavohenke]

But that wouldn't make it harder for humans (aka the contributors) parse that file? :D

[azproduction]

Contributors don't read .jscsrc they fix issues reported by checker. But every time they ready .jscsrc they browse example for each rule. Eg:

Example

"disallowSpacesInFunctionExpression": {
    "beforeOpeningRoundBrace": true,
    "beforeOpeningCurlyBrace": true
}

Valid

function a(){}

Invalid

function a () {}
function a (){}

In other words they translate rule for robots to a valid example for humans. But we can have such example from the very beginning.

[azproduction]

And if you are creating the brand new Code style. The only thing you need is to manually format code reference. It is much easier than look up for suitable rules.

[Rantanen]

Looking at that code style:

• Spaces inside object braces?
• ! operator isn't shown. Does this stick or not?
• No IIFEs
• Type conversions
• ...
• maximumLineLength of 30 or so?

It's a problem of how much can be interpreted from that file. In one extreme an empty ruleset is valid for that file whereas on the other extreme it will result in conflicting rules being enabled, just because the example file doesn't contain cases for them.

Also with such example code style, new rules added to jscs might get enabled automatically and result in the existing code being against style. Say in future there will be a rule to disallow the use of undefined. The current code style file doesn't mention it -> The rule goes active automatically and rejects any real files which use undefined.

The only sane way to handle the rules is specifying them one by one... Now generating the initial .jsonconfig based on an example code style would make sense. But in the end rely on the json for the actual configuration.

[azproduction]

This code is just a brief example of .codestyle.js to illustrate the idea.

The current code style file doesn't mention it -> The rule goes active automatically and rejects any real files which use undefined

As example. We can have function declaration for each rule:

function undefinedExample (a) {
    return typeof a === 'undefined';
}

We can discuss this.

But in the end rely on the json for the actual configuration.

But this destroys .codestyle.js idea.

[Rantanen]

This code is just a brief example of .codestyle.js to illustrate the idea.

So the real .codestyle.js would need to be few hundred lines?

Contributors don't read .jscsrc they fix issues reported by checker. But every time they ready .jscsrc they browse example for each rule. Eg:

With a configuration file of 100+ lines, finding the spot that specifies a rule (and thus has an example of it) becomes equally cumbersome.

But this destroys .codestyle.js idea.

Which is.. ?

• Easier configuration? No it doesn't. You can create the initial .json file from code.
• Contributors having concrete example of the required style? Arguably. While they get an example the example might be overly verbose and ambiguous.

Configuration needs to be:

• Forwards compatible: When JSCS is modified, (real) rules aren't added/modified automatically without explicit changes to the configuration.
• Unambiguous: Everything that needs to be specified should be specified. Everything that isn't specified should be ignored.
• Easy to modify: With .codestyle.js changing a single rule might require changes in several places.
• Resistant to errors: There should be little risk in wrong errors. With .codestyle.js, accidental space at the end of one line (in 100+ line file) would disable disallowTrailingWhitespace rule for the whole project.

What if the two options were combined?

Support .codestyle.js, but instead of deducing the rules from the code, include them in comments. Partial example:

/* jscs: validateIndentation: 2 */
/* jscs: disallowSpacesInFunctionExpression: {
    beforeOpeningRoundBrace: true,
    beforeOpeningCurlyBrace: false
} */
function a() {
  // No space before '(', space after '{'

  /* jscs: requireKeywordsOnNewLine: else */
  /* jscs: requireSpaceAfterKeywords: if, else */
  if (true) {
     //
  }
  else {
     //
  }
}

// Other options
/* jscs: disallowTrailingWhitespace */

This would be...

• Forwards compatible: Just like with .json, rules must be explicitly specified.
• Unambiguous: Every rule is specified.
• Easy to modify: Changing a rule requires changes to one comment.
• Resistant to errors: Accidental typoes or mistakes in the code do not affect the jscs options.
• Self validating: The actual .codestyle.js could be used against itself to validate the contained JavaScript example code.

The main downside is using this format for quickly checking the wanted code style. The jscs comments reduce the actual code readability. But on the other hand, this format better highlights the enforced rules at glance.

[azproduction]

So the real .codestyle.js would need to be few hundred lines?

We can use ES6 imports to import rules:

import {beforeOpeningRoundBrace} from 'http://jscs.org/rules/v1.3.4/common.js';

I like your example of .codestyle.js. And we can also increase readability:

// validate indentation: 2 spaes
// disallow spaces in function expression: before opening round brace
function a() {
  // require keywords on new line: else
  // require space after keywords: if, else
  if (true) {
  }
  else {
  }
}

// disallow trailing whitespace

[markelog]

I guess this idea was suggested to a lot code analysis tools, but the problem with that idea and reason why all, as far i'm aware, those tools has lack of that feature - it's because logic of it is elusive and fuzzy and all&all - pretty hard to implement.

But that said, we can start from the easy part (but still has to deal with a lot of icky stuff) and generate the rules (i.e. .jscsrc) from code example and add it as experimental cli option, then go from there.

Although we have a lot on our plate right now, even above mentioned inline rules, should come up only in 2.1 (as it stands right now).

[sindresorhus]

👍 This would make JSCS a lot more approachable. And it would open up the possibility of running JSCS on a codebase without a .jscs.json config file and have inconsistencies reported because the style is automatically detected. With this and #516 I think JSCS is on good way on making us not ever having to think about code style in JS again. That's huge!

[ericclemmons]

Just stumbled upon this discussion, and not quite sure where I stand. the .jcsrc is the best for configuring the tool, but even after setting it all up, it's still a bit costly for newcomers to configure their own standards or to even adjust existing configs.

I had planned on taking a stab at resolving this dilemma, but didn't consider the .codestyle.js idea...

[benjamine]

what seems to be behind this idea is each rule having it's own method to detect it's setting value based on sample code.
If you have that, then you can not only use a .codestyle.js but also (as @sindresorhus suggest) go forward and use jscs only to detect inconsistencies, something you could run against any js codebase.

processNextJsFile(filename, code) {
   rules.forEach(function(rule) {
      if (rule.isSet) {
         // current behavior
         rule.validate(code);
      } else if (rule.checkConsistency) {
         if (!rule.tryToGuessFromSample) {
           throw new Error('sorry this rule doesn\'t support auto detection: ' + rule.name);
         }
         rule.tryToGuessFromSample(code);
         // if successful, now rule.isSet===true
      }
   });
}

this would allow auto-detection to be gradually (and optionally) added to each rule.

[mikesherov]

Thanks everyone for their input. I'm going to open a new issue here and close this one as it has gotten quite noisy here.