Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sort-keys: Cannot set property 'prevName' of null at SpreadElement #11402

Closed
kristw opened this issue Feb 15, 2019 · 2 comments · Fixed by #11403, WealthWizardsEngineering/hpropagate#8 or DavidKindler/burger-builder#46 · May be fixed by jakeherp/burger-builder#11 or jakeherp/burger-builder#15
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly patch candidate This issue may necessitate a patch release in the next few days regression Something broke rule Relates to ESLint's core rules

Comments

@kristw
Copy link
Contributor

kristw commented Feb 15, 2019

Tell us about your environment

  • ESLint Version: 5.14.0
  • Node Version: 10
  • npm Version:

What parser (default, Babel-ESLint, etc.) are you using?

babel-eslint

Please show your full configuration:

Configuration

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

import UntypedJed from 'jed';
import { TranslatorConfig } from './types';

interface Jed {
  translate(input: string): Jed;
  ifPlural(value: number, plural: string): Jed;
  fetch(...args: any[]): string;
}

const DEFAULT_LANGUAGE_PACK = {
  domain: 'superset',
  locale_data: {
    superset: {
      '': {
        domain: 'superset',
        lang: 'en',
        plural_forms: 'nplurals=1; plural=0',
      },
    },
  },
};

export default class Translator {
  i18n: Jed;

  constructor(config: TranslatorConfig = {}) {
    const { languagePack = DEFAULT_LANGUAGE_PACK } = config;
    this.i18n = new UntypedJed(languagePack) as Jed;
  }

  translate(input: string, ...args: any[]): string {
    return this.i18n.translate(input).fetch(...args);
  }

  translateWithNumber(singular: string, plural: string, num: number = 0, ...args: any[]): string {
    return this.i18n
      .translate(singular)
      .ifPlural(num, plural)
      .fetch(...args);
  }
}
beemo eslint "./packages/superset-ui-translation/{src,test,storybook}/**/Translator.{js,jsx,ts,tsx}"

What did you expect to happen?

Lint completes successfully. It works fine with eslint@5.13.0

What actually happened? Please include the actual, raw output from ESLint.

TypeError: Cannot set property 'prevName' of null
    at SpreadElement (/Users/krist_wongsuphasawat/workspace/superset-ui/node_modules/eslint/lib/rules/sort-keys.js:133:32)
    at listeners.(anonymous function).forEach.listener (/Users/krist_wongsuphasawat/workspace/superset-ui/node_modules/eslint/lib/util/safe-emitter.js:45:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/krist_wongsuphasawat/workspace/superset-ui/node_modules/eslint/lib/util/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/Users/krist_wongsuphasawat/workspace/superset-ui/node_modules/eslint/lib/util/node-event-generator.js:251:26)
    at NodeEventGenerator.applySelectors (/Users/krist_wongsuphasawat/workspace/superset-ui/node_modules/eslint/lib/util/node-event-generator.js:280:22)
    at NodeEventGenerator.enterNode (/Users/krist_wongsuphasawat/workspace/superset-ui/node_modules/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (/Users/krist_wongsuphasawat/workspace/superset-ui/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:632:23)
    at nodeQueue.forEach.traversalInfo (/Users/krist_wongsuphasawat/workspace/superset-ui/node_modules/eslint/lib/linter.js:750:28)
    at Array.forEach (<anonymous>)

See more at Travis
https://travis-ci.com/apache-superset/superset-ui/builds/101145033

Are you willing to submit a pull request to fix this bug?

Just posted #11403

@kristw kristw added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Feb 15, 2019
@not-an-aardvark
Copy link
Member

Thanks for the report, I can reproduce this issue. It seems like sort-keys is just throwing an error whenever a SpreadElement occurs somewhere other than an object literal.

@not-an-aardvark not-an-aardvark added rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion regression Something broke patch candidate This issue may necessitate a patch release in the next few days and removed triage An ESLint team member will look at this issue soon labels Feb 15, 2019
@not-an-aardvark
Copy link
Member

This bug is quickly detected by our fuzzer, but unfortunately we're not running the fuzzer as part of CI. I'm going to work on a PR to enable running the fuzzer in CI, at least to look for rule crashes -- this sort of issue has been happening too often.

not-an-aardvark added a commit that referenced this issue Feb 16, 2019
This commit turns on our existing fuzzer tool to run a small number of iterations when the user runs `npm test`. This is intended to prevent bugs like #11402 where a rule completely breaks when it encounters a particular syntax, but the author doesn't think to test for that kind of syntax.

When there are no fuzzing errors, this adds about 5 seconds to the `npm test` time. (When there are fuzzing errors, it takes more time because the fuzzer does extra work to try to find the minimal config that reproduces the bug.)

The fuzzer actually has two modes: "crash" (where it only tries to detect rule crashes), and "autofix" (where it additionally tries to detect cases where a rule's autofixer creates a syntax error). For now, this PR just enables "crash" mode, because I remember "autofix" mode had some false positives (although they might have been fixed due to parser upgrades).
gajus added a commit to gajus/slonik that referenced this issue Feb 18, 2019
btmills pushed a commit that referenced this issue Feb 18, 2019
* Fix: sort-keys throws Error at SpreadElement (fixes #11402)

* Fix: Add unit test

* Fix: address reviewer's comment
pcorpet added a commit to bayesimpact/docker-react that referenced this issue Feb 18, 2019
aladdin-add pushed a commit that referenced this issue Feb 18, 2019
This commit turns on our existing fuzzer tool to run a small number of iterations when the user runs `npm test`. This is intended to prevent bugs like #11402 where a rule completely breaks when it encounters a particular syntax, but the author doesn't think to test for that kind of syntax.

When there are no fuzzing errors, this adds about 5 seconds to the `npm test` time. (When there are fuzzing errors, it takes more time because the fuzzer does extra work to try to find the minimal config that reproduces the bug.)

The fuzzer actually has two modes: "crash" (where it only tries to detect rule crashes), and "autofix" (where it additionally tries to detect cases where a rule's autofixer creates a syntax error). For now, this PR just enables "crash" mode, because I remember "autofix" mode had some false positives (although they might have been fixed due to parser upgrades).
not-an-aardvark added a commit that referenced this issue Feb 19, 2019
This commit turns on our existing fuzzer tool to run a small number of iterations when the user runs `npm test`. This is intended to prevent bugs like #11402 where a rule completely breaks when it encounters a particular syntax, but the author doesn't think to test for that kind of syntax.

When there are no fuzzing errors, this adds about 5 seconds to the `npm test` time. (When there are fuzzing errors, it takes more time because the fuzzer does extra work to try to find the minimal config that reproduces the bug.)

The fuzzer actually has two modes: "crash" (where it only tries to detect rule crashes), and "autofix" (where it additionally tries to detect cases where a rule's autofixer creates a syntax error). For now, this PR just enables "crash" mode, because I remember "autofix" mode had some false positives (although they might have been fixed due to parser upgrades).
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Aug 18, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.