google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.4k stars 1.15k forks source link

ES6 or CommonJS modules cause "Missing semicolon" errors #2605

Open dbow opened 7 years ago

dbow commented 7 years ago

I'm seeing erroneous WARNING - Missing semicolon errors when using ES6 or CommonJS modules (with jscomp_warning turned on for all checks). The warnings do not appear for Closure Library (goog) modules.

It sounds like Closure Compiler transpiles ES6 and CommonJS to Closure Library modules, so I wonder if a missing semicolon error is introduced during that process?

Here are 3 simple test examples, one for each module system. They each have an entry point test.js file that imports a module. I've included the module and test code for each, as well as the output of the command.

All commands were run with the v20170626 version of the compiler:

$ java -jar ./compiler.jar --version
Closure Compiler (http://github.com/google/closure-compiler)
Version: v20170626
Built on: 2017-06-30 12:25

CommonJS

./module.js

module.exports = 1;

./test.js

const value = require('./module.js');

console.log(value);
$ java -jar ./compiler.jar \
  --jscomp_warning=* \
  --js=./test.js \
  --js=./module.js \
  --entry_point=./test.js
  --process_common_js_modules

./module.js:1: WARNING - Missing semicolon
module.exports = 1;
^^^^^^^^^^^^^^^^^^^

./test.js:1: WARNING - Missing semicolon
const value = require('./module.js');
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0 error(s), 2 warning(s), 100.0% typed
var module$module=1;var value=module$module;console.log(module$module);

Result: 2 Warnings for Missing semicolon on lines that clearly have a semicolon.

ES6 modules

./module.js

export const value = 1;

./test.js

import { value } from './module.js';

console.log(value);
$ java -jar ./compiler.jar \
  --jscomp_warning=* \
  --js=./test.js \
  --js=./module.js \
  --entry_point=./test.js

./module.js:1: WARNING - Missing semicolon
export const value = 1;
^^^^^^^^^^^^^^^^^^^^^^^

./module.js:1: WARNING - Missing semicolon
export const value = 1;
       ^^^^^^^^^^^^^^^

./module.js:1: WARNING - Missing semicolon
export const value = 1;
             ^^^^^^^^^

./test.js:1: WARNING - Missing semicolon
import { value } from './module.js';
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0 error(s), 4 warning(s), 100.0% typed
var module$module={},value$$module$module=1;module$module.value=value$$module$module;var module$test={};console.log(module$module.value);

Result: 4 Warnings for Missing semicolon on lines that clearly have a semicolon.

goog modules

./module.js

goog.module('test.module');

const value = 1;

exports = {value};

./test.js

goog.module('test');

const {value} = goog.require('test.module');

console.log(value);
$ java -jar ./compiler.jar \
  --jscomp_warning=* \
  --js=./test.js \
  --js=./module.js \
  --entry_point=goog:test

var module$exports$test$module={value:1};var module$exports$test={};console.log(module$exports$test$module.value);

Result: No warnings with Closure Library (goog.module).

MatrixFrog commented 7 years ago

--jscomp_off=lintChecks should work as a workaround, but we should definitely find out what's going on with this.

jameskeane commented 6 years ago

@MatrixFrog Is there an easy way to enable lint checks in CompilerTestCase ?

brad4d commented 6 years ago

@jameskeane I think what you really want to know here is how to test the fix you checked into your fork so you can create a good PR from it.

I suggest you create the PR without a test and we continue the discussion of how to correctly test your fix there. Thanks for your work on this issue.

jameskeane commented 6 years ago

@brad4d There are a bunch of spots where the AST transforms result in incorrect lint warnings; I closed the original PR #3035 because ES6 class desugaring was still throwing missing semicolon warnings.

concavelenz commented 6 years ago

We are moving everything except module rewriting later (after the checks)