kellyselden / broccoli-jscs

Broccoli plugin for jscs
MIT License
16 stars 16 forks source link

If I fix one of many issues in a file, it changes to passing without fixing the others - possibly related to dependencies #60

Closed adam-knights closed 8 years ago

adam-knights commented 8 years ago

Today I added the rule "requireEnhancedObjectLiterals": true.

Steps to reproduce:

1) Restart Ember CLI

2) load localhost:4200/tests

3) Browse to a file with multiple failures of this rule, eg:

import Ember from 'ember';

// Inputs
// Radius - The radius of the loading spinner to load
// Message (Optional) - A message to display the loading spinner
// Delay (Optional) - If supplied specifies a delay (in ms) before the loading spinner is shown
// SingleColorMode - If true then the pre loader spinner stays white

export default Ember.Component.extend({
  init() {
    this.set('_wasDelayed', false);

    return this._super(...arguments);
  },
  classNames: ['pre-loader'],
  delayComplete: Ember.computed({
    set(key, val) {
      return val;
    },
    get() {
      let delay = this.get('delay');
      let wasDelayed = this.get('_wasDelayed');

      if (delay && !wasDelayed) {
        this.set('_wasDelayed', true);

        this.startDelayPolling(delay);

        return false;
      }

      return true;
    }
  }),
  pathClass: Ember.computed('singleColorMode', function() {
    if (this.get('singleColorMode')) {
      return 'path path-single';
    }

    return 'path path-colored';
  }),
  startDelayPolling: function(delay) {
    this._poller = Ember.run.later(this, () => {
      this.set('delayComplete', true);
    }, delay);
  },
  stopDelayPolling: function() {
    Ember.run.cancel(this._poller);
  },
  willDestroy: function() {
    this._super(...arguments);
    this.stopDelayPolling();
  },
  size: Ember.computed('viewBoxSize', function() {
    return this.get('viewBoxSize') - 1;
  }),
  viewBoxSize: Ember.computed('strokeWidth', function() {
    return (this.get('radius') * 2) + this.get('strokeWidth');
  }),
  halfViewBoxSize: Ember.computed('viewBoxSize', function() {
    return this.get('viewBoxSize') / 2;
  }),
  strokeWidth: Ember.computed('radius', function() {
    return Math.max(1, Math.round(this.get('radius') / 5));
  })
});

4) Look at the failing test - we see 10+ reasons that this rule is broken.

5) Change one of the occurences, say willDestroy: function() to `willDestroy().

6) Test reloads - it passes, even though there should be further issues.

Here is my .jscsrc file:

{
  "disallowKeywords": ["with"],
  "disallowMixedSpacesAndTabs": true,
  "disallowMultipleVarDecl": "exceptUndefined",
  "disallowNewlineBeforeBlockStatements": true,
  "disallowQuotedKeysInObjects": true,
  "disallowSpaceAfterObjectKeys": true,
  "disallowSpaceAfterPrefixUnaryOperators": true,
  "disallowSpacesInsideParentheses": true,
  "disallowTrailingWhitespace": true,
  "disallowParenthesesAroundArrowParam": true,
  "disallowVar": true,
  "esnext": true,
  "maximumLineLength": 160,
  "requireCamelCaseOrUpperCaseIdentifiers": true,
  "requireCapitalizedComments": true,
  "requireCapitalizedConstructors": true,
  "requireCurlyBraces": true,
  "requireEarlyReturn": true,
  "requireEnhancedObjectLiterals": true,
  "requireSpaceAfterKeywords": [
    "if",
    "else",
    "for",
    "while",
    "do",
    "switch",
    "case",
    "return",
    "try",
    "catch",
    "typeof"
  ],
  "requireSpaceAfterLineComment": true,
  "requireSpaceAfterBinaryOperators": true,
  "requireSpaceBeforeBinaryOperators": true,
  "requireSpaceBeforeBlockStatements": true,
  "requireSpaceBeforeObjectValues": true,
  "requireSpaceBetweenArguments": true,
  "requireSpacesInFunction": {
    "beforeOpeningCurlyBrace": true
  },
  "requireShorthandArrowFunctions": true,
  "validateIndentation": 2,
  "validateParameterSeparator": ", ",
  "validateQuoteMarks": "'"
}

Brocolli jscs v1.2.2, underneath latest jscs 2.9.0.

More info:

7) Restart Ember CLI - test still shows as passing when it should not.

8) Stop CLI, run rmdir /s /q node_modules bower_components dist tmp (windows), npm install, bower install, start cli, test shows up as failing correctly.

So something to do with files made on disk?

Edit 2

Saw this today in a test on a different rule - "maximumLineLength": 160. The test passed even though I was past 160, having fixed a different occurence of it.

This has only started happening in the last couple days, in which I also cleared out my node and bower folders - so could be related to a new release of something brocolli-jscs depends on. So if testing this please pull latest version of all dependencies. But it hand't been long before that when I last cleared them, so specifically a release of a dependency fairly recently?

adam-knights commented 8 years ago

@kellyselden Any more info you would like me to provide?

kellyselden commented 8 years ago

@adam-knights Are you using broccoli-jscs directly or ember-suave? If directly, can you try installing master and see how that works?

adam-knights commented 8 years ago

Directly, i'll try master now. (Incidentally, is directly still recommended?)

kellyselden commented 8 years ago

ember-suave can give you ember specific custom rules, but if you are using the default rules, than directly is perfectly fine.

adam-knights commented 8 years ago

@kellyselden It's slightly inconsistent to reproduce, but I'm 98% sure master does infact fix it. Are you cutting a release soon?

kellyselden commented 8 years ago

@adam-knights yep, when I get some time, probably tonight.

adam-knights commented 8 years ago

Cool, i'll close this now then and if we are in the other 2% i'll reopen.