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

addons-linter fails with errors when using dynamic import syntax #2940

Closed
pdehaan opened this issue Nov 27, 2019 · 5 comments
Closed

addons-linter fails with errors when using dynamic import syntax #2940

pdehaan opened this issue Nov 27, 2019 · 5 comments
Labels
type:syntax Issues related to JavaScript syntax (support).

Comments

@pdehaan
Copy link
Contributor

pdehaan commented Nov 27, 2019

Describe the problem and steps to reproduce it:

I'm trying to run npx addons-linter --self-hosted ./src on https://github.com/mozilla/secure-proxy/ and am getting a few errors.

This is my current output:

npx addons-linter --self-hosted ./src

Validation Summary:

errors          3
notices         0
warnings        45

ERRORS:

Code              Message                   Description                                                                                 File                 Line   Column
JS_SYNTAX_ERROR   JavaScript syntax error   There is a JavaScript syntax error in your code; validation cannot continue on this file.   background/main.js   1      1
JS_SYNTAX_ERROR   JavaScript syntax error   There is a JavaScript syntax error in your code; validation cannot continue on this file.   popup/view.js        7      1
JS_SYNTAX_ERROR   JavaScript syntax error   There is a JavaScript syntax error in your code; validation cannot continue on this file.   popup/popup.js       4      24

WARNINGS:

Code                    Message                                       Description                                                                                       File                             Line    Column
FLAGGED_FILE            Flagged filename found                        Files were found that are either unnecessary or have been included unintentionally. They should   .DS_Store
                                                                      be removed.
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/connectivity.js       8       5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/connectivity.js       13      5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/fxa.js                52      25
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            69      15
UNSUPPORTED_API         captivePortal.canonicalURL is not supported   This API has not been implemented by Firefox.                                                     background/network.js            93      9
UNSUPPORTED_API         captivePortal.canonicalURL is not supported   This API has not been implemented by Firefox.                                                     background/network.js            94      33
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            96      27
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            402     5
UNSUPPORTED_API         browserSettings.ftpProtocolEnabled is not     This API has not been implemented by Firefox.                                                     background/network.js            414     9
                        supported
UNSUPPORTED_API         browserSettings.ftpProtocolEnabled is not     This API has not been implemented by Firefox.                                                     background/network.js            416     7
                        supported
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            419     7
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            433     5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            435     5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            440     5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            442     5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            455     5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/network.js            457     5
UNSUPPORTED_API         captivePortal.canonicalURL is not supported   This API has not been implemented by Firefox.                                                     background/proxyDownChecker.js   16      9
UNSUPPORTED_API         captivePortal.canonicalURL is not supported   This API has not been implemented by Firefox.                                                     background/proxyDownChecker.js   17      38
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/proxyDownChecker.js   19      27
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 47      16
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 338     13
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 343     11
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 356     7
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 362     5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 376     7
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 383     7
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 389     5
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 467     30
UNSUPPORTED_API         experiments.proxyutils is not supported       This API has not been implemented by Firefox.                                                     background/ui.js                 493     5
UNSAFE_VAR_ASSIGNMENT   Unsafe assignment to innerHTML                Due to both security and performance concerns, this may not be set using dynamic values which     commons/template.js              60      5
                                                                      have not been adequately sanitized. This can lead to security issues or fairly serious
                                                                      performance degradation.
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      4996    11
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          eval can be harmful.                          Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      4996    43
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      5506    38
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      23726   17
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      24732   15
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      24851   38
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      31524   38
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      33001   38
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      39066   38
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      40319   38
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      42230   20
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      46175   38
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'
DANGEROUS_EVAL          The Function constructor is eval.             Evaluation of strings as code can lead to security vulnerabilities and performance issues, even   vendor/fxa-crypto-relier.js      46542   38
                                                                      in the most innocuous of circumstances. Please avoid using `eval` and the `Function`
                                                                      constructor when at all possible.'

What happened?

ERRORS:

Code              Message                   Description                                                                                 File                 Line   Column
JS_SYNTAX_ERROR   JavaScript syntax error   There is a JavaScript syntax error in your code; validation cannot continue on this file.   background/main.js   1      1
JS_SYNTAX_ERROR   JavaScript syntax error   There is a JavaScript syntax error in your code; validation cannot continue on this file.   popup/view.js        7      1
JS_SYNTAX_ERROR   JavaScript syntax error   There is a JavaScript syntax error in your code; validation cannot continue on this file.   popup/popup.js       4      24

Where the errors in question are from:

  1. background/main.js:1

    import {ConnectionTester} from "./connection.js";
  2. popup/popup.js:4

    const {View} = await import("./view.js");
  3. popup/view.js:7

    export class View {

What did you expect to happen?

No errors on import / export.

Anything else we should know?

You're awesome.

npx addons-linter --version # 1.14.0
@rpl
Copy link
Member

rpl commented Nov 27, 2019

The scriptType is currently detected from src/scanners/javascript.js here:

detectSourceType(filename) {
// Default options taken from eslint/lib/linter:parse
const parserOptions = {
filePath: filename,
sourceType: 'module',
ecmaVersion: ECMA_VERSION,
};
let sourceType = 'module';
try {
const ast = espree.parse(this.code, parserOptions);
sourceType = this._getSourceType(ast);
} catch (exc) {
sourceType = 'script';
}
return sourceType;
}

If espree.parse throws we do fallback to scriptType "script" but we do not log any message (we should actually log a debug level message at least, to make it simpler to investigate this kind of issues).

What seems to be making espree.parse to throw in this case are not the static import/export (which are supported, and we do have a test case to prevent it from regressing), but the dynamic import (e.g. in src/background/main.js espree.parse throws on line 70 where a dynamic import is being used).

This seems to also been reported upstream in the espree and eslint repos (but both these issues have been closed but the issue is stil there, from a quick look it seems that babel-eslint is the strategy used to workaround the issue at the moment):

@rpl rpl changed the title addons-linter fails with errors when using import/export syntax addons-linter fails with errors when using dynamic import syntax Nov 27, 2019
@pdehaan
Copy link
Contributor Author

pdehaan commented Nov 27, 2019

Except it wasn't just throwing errors on the await import("./view.js") syntax, it was throwing errors for all three of these lines (and only one was a dynamic import()):

// 1. vanilla `import .. from`
import {ConnectionTester} from "./connection.js"; // background/main.js:1

// 2. dynamic `import()`
const {View} = await import("./view.js"); // popup/popup.js:4

// 3. vanilla `export`
export class View { // popup/view.js:7

@rpl
Copy link
Member

rpl commented Nov 28, 2019

The line and column shown in the linter results is not what I'm referring to.

As I mentioned in my previous comment the actual error raised by espree is not logged anywhere, after that error happens we fall back to parse the file as a "script" instead of a "module" and so the linter will mark the first import or export as invalid syntax (which I absolutely agree that it makes this issue way more confusing).

Anyway, the espree errors that triggered the unexpected behavior are all referring to lines with a dynamic import:


To detect the line where the espree error is originally raised, I added the following temporary change locally:

    try {
      const ast = espree.parse(this.code, parserOptions);
      sourceType = this._getSourceType(ast);
    } catch (exc) {
      console.log("DEBUG error detecting sourceType", filename, exc);
      sourceType = 'script';
    }

And, by running it on the secure-proxy sources, I got the following logs for the error that prevented the file to be linted as a "module":

DEBUG error detecting sourceType popup/popup.js { SyntaxError: Unexpected token import
    at Espree.raise (/.../projects/linter/addons-linter/node_modules/espree/lib/espree.js:256:25)
    ... 
    index: 77, lineNumber: 4, column: 24 }

DEBUG error detecting sourceType popup/view.js { SyntaxError: Unexpected token import
    at Espree.raise (/.../projects/linter/addons-linter/node_modules/espree/lib/espree.js:256:25)
    ...
    index: 397, lineNumber: 12, column: 24 }

DEBUG error detecting sourceType background/main.js { SyntaxError: Unexpected token import
    at Espree.raise (/.../projects/linter/addons-linter/node_modules/espree/lib/espree.js:256:25)
    ...
    index: 2288, lineNumber: 70, column: 30 }

aduth added a commit to aduth/StreamLens that referenced this issue Jan 2, 2020
Firefox addons linter does not currently support

See: mozilla/addons-linter#2940

Since migrations are currently limited and this was simply a refactoring, easier to revert and reconsider introducing later, than to fight with Firefox linter.

Reverts b27e135
@stale
Copy link

stale bot commented May 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. If you think this bug should stay open, please comment on the issue with further details. Thank you for your contributions.

@stale stale bot added the state:stale label May 26, 2020
@stale stale bot closed this as completed Jun 9, 2020
@Rob--W Rob--W reopened this Jun 9, 2020
@stale stale bot removed the state:stale label Jun 9, 2020
@willdurand willdurand added the type:syntax Issues related to JavaScript syntax (support). label Jun 23, 2020
@willdurand
Copy link
Member

duplicate of #2498

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:syntax Issues related to JavaScript syntax (support).
Projects
None yet
Development

No branches or pull requests

4 participants