From 75537d9f326252307359027fe220c53eb73fa8ed Mon Sep 17 00:00:00 2001 From: tbreisacher Date: Mon, 23 May 2016 14:43:06 -0700 Subject: [PATCH] Split IDE mode setting into a few different settings since it actually implies a few different things. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=123039034 --- .../jscomp/AbstractCommandLineRunner.java | 2 +- .../javascript/jscomp/AbstractCompiler.java | 5 -- .../javascript/jscomp/CommandLineRunner.java | 1 + .../google/javascript/jscomp/Compiler.java | 17 ++--- .../javascript/jscomp/CompilerOptions.java | 76 ++++++++++++++----- .../jscomp/ControlFlowAnalysis.java | 2 +- .../javascript/jscomp/DefaultPassConfig.java | 8 +- src/com/google/javascript/jscomp/JsAst.java | 5 +- src/com/google/javascript/jscomp/Linter.java | 6 +- .../javascript/jscomp/parsing/Config.java | 17 ++++- .../javascript/jscomp/parsing/IRFactory.java | 8 +- .../jscomp/parsing/ParserRunner.java | 6 +- .../parsing/TypeTransformationParser.java | 1 - .../sourcemap/SourceMapTestCase.java | 3 +- .../lint/CheckMissingSemicolonTest.java | 2 +- .../jscomp/parsing/AttachJsdocsTest.java | 3 +- .../jscomp/parsing/JsDocInfoParserTest.java | 5 +- .../javascript/jscomp/parsing/ParserTest.java | 6 +- 18 files changed, 110 insertions(+), 63 deletions(-) diff --git a/src/com/google/javascript/jscomp/AbstractCommandLineRunner.java b/src/com/google/javascript/jscomp/AbstractCommandLineRunner.java index 4bcdc1513bb..54b6e982ab4 100644 --- a/src/com/google/javascript/jscomp/AbstractCommandLineRunner.java +++ b/src/com/google/javascript/jscomp/AbstractCommandLineRunner.java @@ -1195,7 +1195,7 @@ int processResults(Result result, List modules, B options) throws IOEx outputBundle(); outputModuleGraphJson(); return 0; - } else if (!options.checksOnly && result.success) { + } else if (options.outputJs && result.success) { outputModuleGraphJson(); if (modules == null) { outputSingleBinary(options); diff --git a/src/com/google/javascript/jscomp/AbstractCompiler.java b/src/com/google/javascript/jscomp/AbstractCompiler.java index 1858529f691..8f5caeb82ce 100644 --- a/src/com/google/javascript/jscomp/AbstractCompiler.java +++ b/src/com/google/javascript/jscomp/AbstractCompiler.java @@ -264,11 +264,6 @@ LifeCycleStage getLifeCycleStage() { /** Passes that do cross-scope modifications use this (eg, InlineVariables) */ abstract void reportChangeToEnclosingScope(Node n); - /** - * Returns true if compiling in IDE mode. - */ - abstract boolean isIdeMode(); - /** * Represents the different contexts for which the compiler could have * distinct configurations. diff --git a/src/com/google/javascript/jscomp/CommandLineRunner.java b/src/com/google/javascript/jscomp/CommandLineRunner.java index b227f2e9420..f93622b645e 100644 --- a/src/com/google/javascript/jscomp/CommandLineRunner.java +++ b/src/com/google/javascript/jscomp/CommandLineRunner.java @@ -1496,6 +1496,7 @@ protected CompilerOptions createOptions() { options.setEnvironment(flags.environment); options.setChecksOnly(flags.checksOnly); + options.setOutputJs(!flags.checksOnly); if (flags.useTypesForOptimization) { level.setTypeBasedOptimizationOptions(options); diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index 9f9ea0548ee..32514c79a55 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -712,8 +712,7 @@ private void compileInternal() { return; } - // IDE-mode is defined to stop here, before the heavy rewriting begins. - if (!options.checksOnly && !options.ideMode) { + if (!options.checksOnly) { optimize(); } } @@ -2112,11 +2111,6 @@ public CodingConvention getCodingConvention() { return convention; } - @Override - public boolean isIdeMode() { - return options.ideMode; - } - @Override Config getParserConfig(ConfigContext context) { if (parserConfig == null) { @@ -2160,7 +2154,8 @@ Config getParserConfig(ConfigContext context) { protected Config createConfig(Config.LanguageMode mode) { Config config = ParserRunner.createConfig(mode, options.extraAnnotationNames); - config.setIdeMode(isIdeMode()); + config.setPreserveDetailedSourceInfo(options.preservesDetailedSourceInfo()); + config.setKeepGoing(options.canKeepGoing()); config.setParseJsDocDocumentation(options.isParseJsDocDocumentation()); config.setPreserveJsDocWhitespace(options.isPreserveJsDocWhitespace()); return config; @@ -2236,7 +2231,7 @@ public int getWarningCount() { @Override boolean hasHaltingErrors() { - return !isIdeMode() && getErrorCount() > 0; + return !getOptions().canKeepGoing() && getErrorCount() > 0; } /** @@ -2671,7 +2666,7 @@ public static String getReleaseDate() { @Override void addComments(String filename, List comments) { - if (!isIdeMode()) { + if (!getOptions().preservesDetailedSourceInfo()) { throw new UnsupportedOperationException( "addComments may only be called in IDE mode."); } @@ -2680,7 +2675,7 @@ void addComments(String filename, List comments) { @Override public List getComments(String filename) { - if (!isIdeMode()) { + if (!getOptions().preservesDetailedSourceInfo()) { throw new UnsupportedOperationException( "getComments may only be called in IDE mode."); } diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java index 84de336ffb4..a4238c1c615 100644 --- a/src/com/google/javascript/jscomp/CompilerOptions.java +++ b/src/com/google/javascript/jscomp/CompilerOptions.java @@ -105,26 +105,15 @@ public void setInferConst(boolean value) { */ private boolean assumeStrictThis; - /** - * Configures the compiler for use as an IDE backend. In this mode: - *
    - *
  • No optimization passes will run.
    • - *
  • The last time custom passes are invoked is - * {@link CustomPassExecutionTime#BEFORE_OPTIMIZATIONS}
    • - *
  • The compiler will always try to process all inputs fully, even - * if it encounters errors.
    • - *
  • The compiler may record more information than is strictly - * needed for codegen.
    • - *
- */ - public boolean ideMode; + private boolean allowHotswapReplaceScript = false; + private boolean preserveDetailedSourceInfo = false; + private boolean keepGoing = false; private boolean parseJsDocDocumentation = false; private boolean preserveJsDocWhitespace = false; /** - * Even if checkTypes is disabled, clients might want to still infer types. - * This is mostly used when ideMode is enabled. + * Even if checkTypes is disabled, clients such as IDEs might want to still infer types. */ boolean inferTypes; @@ -730,6 +719,8 @@ public void setAppNameStr(String appNameStr) { public boolean checksOnly; + public boolean outputJs; + public boolean generateExports; // TODO(dimvar): generate-exports should always run after typechecking. @@ -1094,6 +1085,7 @@ public CompilerOptions() { appNameStr = ""; recordFunctionInformation = false; checksOnly = false; + outputJs = true; generateExports = false; generateExportsAfterTypeChecking = true; exportLocalPropertyDefinitions = false; @@ -1513,6 +1505,10 @@ public void setChecksOnly(boolean checksOnly) { this.checksOnly = checksOnly; } + public void setOutputJs(boolean outputJs) { + this.outputJs = outputJs; + } + public void setGenerateExports(boolean generateExports) { this.generateExports = generateExports; } @@ -1808,8 +1804,54 @@ public void setPropertyInvalidationErrors( ImmutableMap.copyOf(propertyInvalidationErrors); } + /** + * Configures the compiler for use as an IDE backend. In this mode: + *
    + *
  • No optimization passes will run.
    • + *
  • The last time custom passes are invoked is + * {@link CustomPassExecutionTime#BEFORE_OPTIMIZATIONS}
    • + *
  • The compiler will always try to process all inputs fully, even + * if it encounters errors.
    • + *
  • The compiler may record more information than is strictly + * needed for codegen.
    • + *
+ * + * @deprecated Some "IDE" clients will need some of these options but not + * others. Consider calling setChecksOnly, setAllowRecompilation, etc, + * explicitly, instead of calling this method which does a variety of + * different things. + */ + @Deprecated public void setIdeMode(boolean ideMode) { - this.ideMode = ideMode; + setChecksOnly(ideMode); + setKeepGoing(ideMode); + setAllowHotswapReplaceScript(ideMode); + setPreserveDetailedSourceInfo(ideMode); + setParseJsDocDocumentation(ideMode); + } + + void setAllowHotswapReplaceScript(boolean allowRecompilation) { + this.allowHotswapReplaceScript = allowRecompilation; + } + + boolean allowsHotswapReplaceScript() { + return allowHotswapReplaceScript; + } + + public void setPreserveDetailedSourceInfo(boolean preserveDetailedSourceInfo) { + this.preserveDetailedSourceInfo = preserveDetailedSourceInfo; + } + + boolean preservesDetailedSourceInfo() { + return preserveDetailedSourceInfo; + } + + void setKeepGoing(boolean keepGoing) { + this.keepGoing = keepGoing; + } + + boolean canKeepGoing() { + return keepGoing; } /** @@ -1829,7 +1871,7 @@ public void setParseJsDocDocumentation(boolean parseJsDocDocumentation) { * @return True when JSDoc documentation will be parsed, false if not. */ public boolean isParseJsDocDocumentation() { - return this.ideMode || this.parseJsDocDocumentation; + return this.parseJsDocDocumentation; } /** diff --git a/src/com/google/javascript/jscomp/ControlFlowAnalysis.java b/src/com/google/javascript/jscomp/ControlFlowAnalysis.java index 2b1cb0eeed7..2419ded6e91 100644 --- a/src/com/google/javascript/jscomp/ControlFlowAnalysis.java +++ b/src/com/google/javascript/jscomp/ControlFlowAnalysis.java @@ -588,7 +588,7 @@ private void handleBreak(Node node) { lastJump = cur; } if (parent == null) { - if (compiler.isIdeMode()) { + if (compiler.getOptions().canKeepGoing()) { // In IDE mode, we expect that the data flow graph may // not be well-formed. return; diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java index dba0467b860..fe2275e2598 100644 --- a/src/com/google/javascript/jscomp/DefaultPassConfig.java +++ b/src/com/google/javascript/jscomp/DefaultPassConfig.java @@ -156,7 +156,7 @@ public DefaultPassConfig(CompilerOptions options) { // wrap them in a function call that is stripped later, this shouldn't // be done in IDE mode where AST changes may be unexpected. protectHiddenSideEffects = options != null && - options.protectHiddenSideEffects && !options.ideMode; + options.protectHiddenSideEffects && !options.allowsHotswapReplaceScript(); } @Override @@ -178,7 +178,7 @@ PreprocessorSymbolTable getPreprocessorSymbolTable() { } void maybeInitializePreprocessorSymbolTable(AbstractCompiler compiler) { - if (options.ideMode) { + if (options.preservesDetailedSourceInfo()) { Node root = compiler.getRoot(); if (preprocessorSymbolTable == null || preprocessorSymbolTable.getRootNode() != root) { @@ -188,7 +188,7 @@ void maybeInitializePreprocessorSymbolTable(AbstractCompiler compiler) { } void maybeInitializeModuleRewriteState() { - if (options.ideMode && this.moduleRewriteState == null) { + if (options.allowsHotswapReplaceScript() && this.moduleRewriteState == null) { this.moduleRewriteState = new ClosureRewriteModule.GlobalRewriteState(); } } @@ -370,7 +370,7 @@ protected List getChecks() { // We assume that only IDE-mode clients will try to query the // typed scope creator after the compile job. - if (!options.ideMode) { + if (!options.preservesDetailedSourceInfo()) { checks.add(clearTypedScopePass); } } diff --git a/src/com/google/javascript/jscomp/JsAst.java b/src/com/google/javascript/jscomp/JsAst.java index c7250b2bbb9..d188995cf06 100644 --- a/src/com/google/javascript/jscomp/JsAst.java +++ b/src/com/google/javascript/jscomp/JsAst.java @@ -150,7 +150,7 @@ private void parse(AbstractCompiler compiler) { root = result.ast; features = result.features; - if (compiler.isIdeMode()) { + if (compiler.getOptions().preservesDetailedSourceInfo()) { compiler.addComments(sourceFile.getName(), result.comments); } } catch (IOException e) { @@ -165,7 +165,8 @@ private void parse(AbstractCompiler compiler) { // Note: we use the ErrorManager here rather than the ErrorReporter as // we don't want to fail if the error was excluded by a warning guard, conversely // we do want to fail if a warning was promoted to an error. - || (errorManager.getErrorCount() > startErrorCount && !compiler.isIdeMode())) { + || (errorManager.getErrorCount() > startErrorCount + && !compiler.getOptions().canKeepGoing())) { // There was a parse error or IOException, so use a dummy block. diff --git a/src/com/google/javascript/jscomp/Linter.java b/src/com/google/javascript/jscomp/Linter.java index 7b4d1e1c22a..d60dc449aa4 100644 --- a/src/com/google/javascript/jscomp/Linter.java +++ b/src/com/google/javascript/jscomp/Linter.java @@ -62,7 +62,11 @@ private static void lint(Path path, boolean fix) throws IOException { // in LintPassConfig can all handle untranspiled ES6. options.setSkipTranspilationAndCrash(true); - options.setIdeMode(true); + options.setPreserveDetailedSourceInfo(true); + + // TODO(tbreisacher): Remove this so that people don't miss parse errors. + options.setKeepGoing(true); + options.setCodingConvention(new GoogleCodingConvention()); // Even though we're not running the typechecker, enable the checkTypes DiagnosticGroup, since diff --git a/src/com/google/javascript/jscomp/parsing/Config.java b/src/com/google/javascript/jscomp/parsing/Config.java index 01fe520746b..587fa004fef 100644 --- a/src/com/google/javascript/jscomp/parsing/Config.java +++ b/src/com/google/javascript/jscomp/parsing/Config.java @@ -50,9 +50,14 @@ public enum LanguageMode { boolean preserveJsDocWhitespace; /** - * Whether we're in IDE mode. + * Whether to keep detailed source location information such as the exact length of every node. */ - boolean isIdeMode; + boolean preserveDetailedSourceInfo; + + /** + * Whether to keep going after encountering a parse error. + */ + boolean keepGoing; /** * Recognized JSDoc annotations, mapped from their name to their internal @@ -96,8 +101,12 @@ private static Map buildAnnotationNames( return annotationBuilder.build(); } - public void setIdeMode(boolean isIdeMode) { - this.isIdeMode = isIdeMode; + public void setPreserveDetailedSourceInfo(boolean preserveDetailedSourceInfo) { + this.preserveDetailedSourceInfo = preserveDetailedSourceInfo; + } + + public void setKeepGoing(boolean keepGoing) { + this.keepGoing = keepGoing; } public void setParseJsDocDocumentation(boolean parseJsDocDocumentation) { diff --git a/src/com/google/javascript/jscomp/parsing/IRFactory.java b/src/com/google/javascript/jscomp/parsing/IRFactory.java index 22dab5f2919..74ff36dad20 100644 --- a/src/com/google/javascript/jscomp/parsing/IRFactory.java +++ b/src/com/google/javascript/jscomp/parsing/IRFactory.java @@ -931,13 +931,13 @@ private JSDocInfo parseInlineTypeDoc(Comment node) { // Set the length on the node if we're in IDE mode. void maybeSetLength( Node node, SourcePosition start, SourcePosition end) { - if (config.isIdeMode) { + if (config.preserveDetailedSourceInfo) { node.setLength(end.offset - start.offset); } } void maybeSetLengthFrom(Node node, Node ref) { - if (config.isIdeMode) { + if (config.preserveDetailedSourceInfo) { node.setLength(ref.getLength()); } } @@ -1253,9 +1253,9 @@ Node processFunction(FunctionDeclarationTree functionTree) { Node bodyNode = transform(functionTree.functionBody); if (!isArrow && !isSignature && !bodyNode.isBlock()) { - // When in ideMode the parser tries to parse some constructs the + // When in "keep going" mode the parser tries to parse some constructs the // compiler doesn't support, repair it here. - Preconditions.checkState(config.isIdeMode); + Preconditions.checkState(config.keepGoing); bodyNode = IR.block(); } parseDirectives(bodyNode); diff --git a/src/com/google/javascript/jscomp/parsing/ParserRunner.java b/src/com/google/javascript/jscomp/parsing/ParserRunner.java index 7b4c983a5ac..c6e356e1fdf 100644 --- a/src/com/google/javascript/jscomp/parsing/ParserRunner.java +++ b/src/com/google/javascript/jscomp/parsing/ParserRunner.java @@ -92,7 +92,7 @@ public static ParseResult parse( // TODO(johnlenz): unify "SourceFile", "Es6ErrorReporter" and "Config" SourceFile file = new SourceFile(sourceFile.getName(), sourceString); Es6ErrorReporter es6ErrorReporter = - new Es6ErrorReporter(errorReporter, config.isIdeMode); + new Es6ErrorReporter(errorReporter, config.keepGoing); com.google.javascript.jscomp.parsing.parser.Parser.Config es6config = new com.google.javascript.jscomp.parsing.parser.Parser.Config(mode( config.languageMode)); @@ -101,7 +101,7 @@ public static ParseResult parse( Node root = null; List comments = ImmutableList.of(); FeatureSet features = p.getFeatures(); - if (tree != null && (!es6ErrorReporter.hadError() || config.isIdeMode)) { + if (tree != null && (!es6ErrorReporter.hadError() || config.preserveDetailedSourceInfo)) { IRFactory factory = IRFactory.transformTree(tree, sourceFile, sourceString, config, errorReporter); root = factory.getResultNode(); @@ -109,7 +109,7 @@ public static ParseResult parse( root.setIsSyntheticBlock(true); root.putProp(Node.FEATURE_SET, features); - if (config.isIdeMode) { + if (config.preserveDetailedSourceInfo) { comments = p.getComments(); } } diff --git a/src/com/google/javascript/jscomp/parsing/TypeTransformationParser.java b/src/com/google/javascript/jscomp/parsing/TypeTransformationParser.java index 768a6006db2..e488edd71f4 100644 --- a/src/com/google/javascript/jscomp/parsing/TypeTransformationParser.java +++ b/src/com/google/javascript/jscomp/parsing/TypeTransformationParser.java @@ -250,7 +250,6 @@ private boolean checkParameterCount(Node expr, Keywords keyword) { public boolean parseTypeTransformation() { Config config = new Config(new HashSet(), new HashSet(), LanguageMode.ECMASCRIPT6); - config.setIdeMode(true); // TODO(lpino): ParserRunner reports errors if the expression is not // ES6 valid. We need to abort the validation of the type transformation // whenever an error is reported. diff --git a/test/com/google/debugging/sourcemap/SourceMapTestCase.java b/test/com/google/debugging/sourcemap/SourceMapTestCase.java index 9a9507e2291..df77befe84b 100644 --- a/test/com/google/debugging/sourcemap/SourceMapTestCase.java +++ b/test/com/google/debugging/sourcemap/SourceMapTestCase.java @@ -283,8 +283,7 @@ protected RunResult compile( Compiler compiler = new Compiler(); CompilerOptions options = getCompilerOptions(); - // Turn on IDE mode to get rid of optimizations. - options.ideMode = true; + options.setChecksOnly(true); List inputs = ImmutableList.of(SourceFile.fromCode(fileName1, js1)); diff --git a/test/com/google/javascript/jscomp/lint/CheckMissingSemicolonTest.java b/test/com/google/javascript/jscomp/lint/CheckMissingSemicolonTest.java index 24ec7b8c60d..1c468d6bf82 100644 --- a/test/com/google/javascript/jscomp/lint/CheckMissingSemicolonTest.java +++ b/test/com/google/javascript/jscomp/lint/CheckMissingSemicolonTest.java @@ -32,7 +32,7 @@ public CompilerPass getProcessor(Compiler compiler) { protected CompilerOptions getOptions(CompilerOptions options) { super.getOptions(options); options.setWarningLevel(DiagnosticGroups.LINT_CHECKS, CheckLevel.WARNING); - options.setIdeMode(true); + options.setPreserveDetailedSourceInfo(true); return options; } diff --git a/test/com/google/javascript/jscomp/parsing/AttachJsdocsTest.java b/test/com/google/javascript/jscomp/parsing/AttachJsdocsTest.java index b28e8511aab..fd0232635b3 100644 --- a/test/com/google/javascript/jscomp/parsing/AttachJsdocsTest.java +++ b/test/com/google/javascript/jscomp/parsing/AttachJsdocsTest.java @@ -789,7 +789,8 @@ public void testInlineInExport() { private Node parse(String source, String... warnings) { TestErrorReporter testErrorReporter = new TestErrorReporter(null, warnings); Config config = ParserRunner.createConfig(mode, null); - config.setIdeMode(true); + config.setPreserveDetailedSourceInfo(true); + config.setKeepGoing(true); config.setParseJsDocDocumentation(true); Node script = ParserRunner.parse( new SimpleSourceFile("input", false), diff --git a/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java b/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java index 0bba1951839..6a75d2127f9 100644 --- a/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java +++ b/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java @@ -4375,7 +4375,8 @@ private Node parseFull(String code, String... warnings) { TestErrorReporter testErrorReporter = new TestErrorReporter(null, warnings); Config config = new Config(extraAnnotations, extraSuppressions, LanguageMode.ECMASCRIPT3); - config.setIdeMode(true); + config.setPreserveDetailedSourceInfo(true); + config.setKeepGoing(true); config.setParseJsDocDocumentation(true); ParseResult result = ParserRunner.parse( @@ -4412,9 +4413,7 @@ private JSDocInfo parse(String comment, boolean parseDocumentation, boolean parseFileOverview, boolean preserveWhitespace, String... warnings) { TestErrorReporter errorReporter = new TestErrorReporter(null, warnings); - boolean isIdeMode = parseDocumentation; Config config = new Config(extraAnnotations, extraSuppressions, LanguageMode.ECMASCRIPT3); - config.setIdeMode(isIdeMode); config.setParseJsDocDocumentation(parseDocumentation); config.setPreserveJsDocWhitespace(preserveWhitespace); StaticSourceFile file = new SimpleSourceFile("testcode", false); diff --git a/test/com/google/javascript/jscomp/parsing/ParserTest.java b/test/com/google/javascript/jscomp/parsing/ParserTest.java index 2159cb6b430..57774db8af4 100644 --- a/test/com/google/javascript/jscomp/parsing/ParserTest.java +++ b/test/com/google/javascript/jscomp/parsing/ParserTest.java @@ -3175,7 +3175,8 @@ private Node regex(String regex) { private Node parseError(String source, String... errors) { TestErrorReporter testErrorReporter = new TestErrorReporter(errors, null); Config config = ParserRunner.createConfig(mode, null); - config.setIdeMode(isIdeMode); + config.setPreserveDetailedSourceInfo(isIdeMode); + config.setKeepGoing(isIdeMode); config.setParseJsDocDocumentation(isIdeMode); ParseResult result = ParserRunner.parse( new SimpleSourceFile("input", false), @@ -3205,7 +3206,8 @@ private Node parseWarning(String string, String... warnings) { TestErrorReporter testErrorReporter = new TestErrorReporter(null, warnings); StaticSourceFile file = new SimpleSourceFile("input", false); Config config = ParserRunner.createConfig(mode, null); - config.setIdeMode(isIdeMode); + config.setPreserveDetailedSourceInfo(isIdeMode); + config.setKeepGoing(isIdeMode); config.setParseJsDocDocumentation(isIdeMode); ParserRunner.ParseResult result = ParserRunner.parse( file,