From 7e3af08a097c024ddbc6c2af1e0ff31d2a8b742c Mon Sep 17 00:00:00 2001 From: Andy Date: Mon, 14 May 2018 12:00:40 -0700 Subject: [PATCH] Don't add a suggestion to convert to an es6 module if no commonjs import/export appears at top-level. (#24101) --- src/services/suggestionDiagnostics.ts | 27 ++++++++++++++++++- ...efactorConvertToEs6Module_notAtTopLevel.ts | 12 +++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/refactorConvertToEs6Module_notAtTopLevel.ts diff --git a/src/services/suggestionDiagnostics.ts b/src/services/suggestionDiagnostics.ts index 0c8ec58798..f2961e1934 100644 --- a/src/services/suggestionDiagnostics.ts +++ b/src/services/suggestionDiagnostics.ts @@ -5,7 +5,9 @@ namespace ts { const checker = program.getDiagnosticsProducingTypeChecker(); const diags: Diagnostic[] = []; - if (sourceFile.commonJsModuleIndicator && (programContainsEs6Modules(program) || compilerOptionsIndicateEs6Modules(program.getCompilerOptions()))) { + if (sourceFile.commonJsModuleIndicator && + (programContainsEs6Modules(program) || compilerOptionsIndicateEs6Modules(program.getCompilerOptions())) && + containsTopLevelCommonjs(sourceFile)) { diags.push(createDiagnosticForNode(getErrorNodeFromCommonJsIndicator(sourceFile.commonJsModuleIndicator), Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module)); } @@ -61,6 +63,29 @@ namespace ts { return diags.concat(checker.getSuggestionDiagnostics(sourceFile)); } + // convertToEs6Module only works on top-level, so don't trigger it if commonjs code only appears in nested scopes. + function containsTopLevelCommonjs(sourceFile: SourceFile): boolean { + return sourceFile.statements.some(statement => { + switch (statement.kind) { + case SyntaxKind.VariableStatement: + return (statement as VariableStatement).declarationList.declarations.some(decl => + isRequireCall(propertyAccessLeftHandSide(decl.initializer), /*checkArgumentIsStringLiteralLike*/ true)); + case SyntaxKind.ExpressionStatement: { + const { expression } = statement as ExpressionStatement; + if (!isBinaryExpression(expression)) return isRequireCall(expression, /*checkArgumentIsStringLiteralLike*/ true); + const kind = getSpecialPropertyAssignmentKind(expression); + return kind === SpecialPropertyAssignmentKind.ExportsProperty || kind === SpecialPropertyAssignmentKind.ModuleExports; + } + default: + return false; + } + }); + } + + function propertyAccessLeftHandSide(node: Expression): Expression { + return isPropertyAccessExpression(node) ? propertyAccessLeftHandSide(node.expression) : node; + } + function importNameForConvertToDefaultImport(node: AnyValidImportOrReExport): Identifier | undefined { switch (node.kind) { case SyntaxKind.ImportDeclaration: diff --git a/tests/cases/fourslash/refactorConvertToEs6Module_notAtTopLevel.ts b/tests/cases/fourslash/refactorConvertToEs6Module_notAtTopLevel.ts new file mode 100644 index 0000000000..8de3581388 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToEs6Module_notAtTopLevel.ts @@ -0,0 +1,12 @@ +/// + +// @allowJs: true +// @target: esnext + +// @Filename: /a.js +////(function() { +//// module.exports = 0; +////})(); + +verify.getSuggestionDiagnostics([]); +