Added new configuration option "reportDuplicateImports" that generates an error or warning when a symbol or module is imported more than once in a source file.

client/schemas/pyrightconfig.schema.json

@@ -132,6 +132,12 @@
       "title": "Controls reporting of local variables that are not accessed",
       "default": "none"
     },
+    "reportDuplicateImport": {
+      "$id": "#/properties/reportDuplicateImport",
+      "$ref": "#/definitions/diagnostic",
+      "title": "Controls reporting of symbols or modules that are imported more than once",
+      "default": "none"
+    },
     "reportOptionalSubscript": {
       "$id": "#/properties/reportOptionalSubscript",
       "$ref": "#/definitions/diagnostic",

docs/configuration.md

@@ -56,6 +56,8 @@ The following settings control pyright’s diagnostic output (warnings or errors
 
 **reportUnusedVariable** [boolean or string, optional]: Generate or suppress diagnostics for a variable that is not accessed. The default value for this setting is 'none'.
 
+**reportDuplicateImport** [boolean or string, optional]: Generate or suppress diagnostics for an imported symbol or module that is imported more than once.
+
 **reportOptionalSubscript** [boolean or string, optional]: Generate or suppress diagnostics for an attempt to subscript (index) a variable with an Optional type. The default value for this setting is 'none'.
 
 **reportOptionalMemberAccess** [boolean or string, optional]: Generate or suppress diagnostics for an attempt to access a member of a variable with an Optional type. The default value for this setting is 'none'.

server/src/analyzer/checker.ts

@@ -19,22 +19,23 @@ import { Diagnostic, DiagnosticAddendum } from '../common/diagnostic';
 import { DiagnosticRule } from '../common/diagnosticRules';
 import { TextRange } from '../common/textRange';
 import { AssertNode, AssignmentExpressionNode, AssignmentNode, AugmentedAssignmentNode,
-    BinaryOperationNode, CallNode, ClassNode, DelNode, ErrorNode, ExceptNode, ExpressionNode,
-    FormatStringNode, ForNode, FunctionNode, IfNode, ImportAsNode, ImportFromNode, IndexNode,
-    LambdaNode, ListComprehensionNode, MemberAccessNode, ModuleNode, NameNode, ParameterCategory,
-    ParseNode, ParseNodeType, RaiseNode, ReturnNode, SliceNode, StringListNode, SuiteNode,
-    TernaryNode, TupleNode, TypeAnnotationNode, UnaryOperationNode, UnpackNode, WhileNode,
-    WithNode, YieldFromNode, YieldNode } from '../parser/parseNodes';
+    BinaryOperationNode, CallNode, ClassNode, DelNode, ErrorNode, ExceptNode,
+    FormatStringNode, ForNode, FunctionNode, IfNode, ImportAsNode, ImportFromAsNode, ImportFromNode,
+    IndexNode, LambdaNode, ListComprehensionNode, MemberAccessNode, ModuleNode, NameNode,
+    ParameterCategory, ParseNode, ParseNodeType, RaiseNode, ReturnNode, SliceNode, StringListNode,
+    SuiteNode, TernaryNode, TupleNode, TypeAnnotationNode, UnaryOperationNode, UnpackNode,
+    WhileNode, WithNode, YieldFromNode, YieldNode } from '../parser/parseNodes';
 import { AnalyzerFileInfo } from './analyzerFileInfo';
 import * as AnalyzerNodeInfo from './analyzerNodeInfo';
 import { Declaration, DeclarationType } from './declaration';
+import { getTopLevelImports } from './importStatementUtils';
 import * as ParseTreeUtils from './parseTreeUtils';
 import { ParseTreeWalker } from './parseTreeWalker';
 import { ScopeType } from './scope';
 import { Symbol } from './symbol';
 import * as SymbolNameUtils from './symbolNameUtils';
 import { getLastTypedDeclaredForSymbol } from './symbolUtils';
-import { EvaluatorFlags, TypeEvaluator } from './typeEvaluator';
+import { TypeEvaluator } from './typeEvaluator';
 import { ClassType, combineTypes, FunctionType, isAnyOrUnknown, isNoneOrNever, isTypeSame,
     NoneType, ObjectType, Type, TypeCategory, UnknownType } from './types';
 import { containsUnknown, derivesFromClassRecursive, doForSubtypes,
@@ -67,6 +68,8 @@ export class Checker extends ParseTreeWalker {
         // Perform a one-time validation of symbols in all scopes
         // defined in this module for things like unaccessed variables.
         this._validateSymbolTables();
+
+        this._reportDuplicateImports();
     }
 
     walk(node: ParseNode) {
@@ -1385,4 +1388,46 @@ export class Checker extends ParseTreeWalker {
             }
         }
     }
+
+    private _reportDuplicateImports() {
+        const importStatements = getTopLevelImports(this._moduleNode);
+
+        const importModuleMap = new Map<string, ImportAsNode>();
+
+        importStatements.orderedImports.forEach(importStatement => {
+            if (importStatement.node.nodeType === ParseNodeType.ImportFrom) {
+                const symbolMap = new Map<string, ImportFromAsNode>();
+
+                importStatement.node.imports.forEach(importFromAs => {
+                    // Ignore duplicates if they're aliased.
+                    if (!importFromAs.alias) {
+                        const prevImport = symbolMap.get(importFromAs.name.value);
+                        if (prevImport) {
+                            this._evaluator.addDiagnostic(
+                                this._fileInfo.diagnosticSettings.reportDuplicateImport,
+                                DiagnosticRule.reportDuplicateImport,
+                                `'${ importFromAs.name.value }' is imported more than once`,
+                                importFromAs.name);
+                        } else {
+                            symbolMap.set(importFromAs.name.value, importFromAs);
+                        }
+                    }
+                });
+            } else if (importStatement.subnode) {
+                // Ignore duplicates if they're aliased.
+                if (!importStatement.subnode.alias) {
+                    const prevImport = importModuleMap.get(importStatement.moduleName);
+                    if (prevImport) {
+                        this._evaluator.addDiagnostic(
+                            this._fileInfo.diagnosticSettings.reportDuplicateImport,
+                            DiagnosticRule.reportDuplicateImport,
+                            `'${ importStatement.moduleName }' is imported more than once`,
+                            importStatement.subnode);
+                    } else {
+                        importModuleMap.set(importStatement.moduleName, importStatement.subnode);
+                    }
+                }
+            }
+        });
+    }
 }

server/src/common/configOptions.ts

@@ -79,6 +79,9 @@ export interface DiagnosticSettings {
     // Report variable that is not accessed?
     reportUnusedVariable: DiagnosticLevel;
 
+    // Report symbol or module that is imported more than once?
+    reportDuplicateImport: DiagnosticLevel;
+
     // Report attempts to subscript (index) an Optional type?
     reportOptionalSubscript: DiagnosticLevel;
 
@@ -181,6 +184,7 @@ export function getDiagLevelSettings() {
         DiagnosticRule.reportUnusedClass,
         DiagnosticRule.reportUnusedFunction,
         DiagnosticRule.reportUnusedVariable,
+        DiagnosticRule.reportDuplicateImport,
         DiagnosticRule.reportOptionalSubscript,
         DiagnosticRule.reportOptionalMemberAccess,
         DiagnosticRule.reportOptionalCall,
@@ -220,6 +224,7 @@ export function getStrictDiagnosticSettings(): DiagnosticSettings {
         reportUnusedClass: 'error',
         reportUnusedFunction: 'error',
         reportUnusedVariable: 'error',
+        reportDuplicateImport: 'error',
         reportOptionalSubscript: 'error',
         reportOptionalMemberAccess: 'error',
         reportOptionalCall: 'error',
@@ -261,6 +266,7 @@ export function getDefaultDiagnosticSettings(): DiagnosticSettings {
         reportUnusedClass: 'none',
         reportUnusedFunction: 'none',
         reportUnusedVariable: 'none',
+        reportDuplicateImport: 'none',
         reportOptionalSubscript: 'none',
         reportOptionalMemberAccess: 'none',
         reportOptionalCall: 'none',
@@ -520,6 +526,11 @@ export class ConfigOptions {
                 configObj.reportUnusedVariable, DiagnosticRule.reportUnusedVariable,
                 defaultSettings.reportUnusedVariable),
 
+            // Read the "reportDuplicateImport" entry.
+            reportDuplicateImport: this._convertDiagnosticLevel(
+                configObj.reportDuplicateImport, DiagnosticRule.reportDuplicateImport,
+                defaultSettings.reportDuplicateImport),
+
             // Read the "reportMissingTypeStubs" entry.
             reportMissingTypeStubs: this._convertDiagnosticLevel(
                 configObj.reportMissingTypeStubs, DiagnosticRule.reportMissingTypeStubs,

server/src/common/diagnosticRules.ts

@@ -22,6 +22,7 @@ export const enum DiagnosticRule {
     reportUnusedClass = 'reportUnusedClass',
     reportUnusedFunction = 'reportUnusedFunction',
     reportUnusedVariable = 'reportUnusedVariable',
+    reportDuplicateImport = 'reportDuplicateImport',
     reportOptionalSubscript = 'reportOptionalSubscript',
     reportOptionalMemberAccess = 'reportOptionalMemberAccess',
     reportOptionalCall = 'reportOptionalCall',

server/src/tests/checker.test.ts

@@ -945,3 +945,16 @@ test('CallSite2', () => {
     const analysisResults = TestUtils.typeAnalyzeSampleFiles(['callSite2.py']);
     validateResults(analysisResults, 0);
 });
+
+test('DuplicateImports1', () => {
+    const configOptions = new ConfigOptions('.');
+
+    // By default, optional diagnostics are ignored.
+    let analysisResults = TestUtils.typeAnalyzeSampleFiles(['duplicateImports1.py'], configOptions);
+    validateResults(analysisResults, 0);
+
+    // Turn on errors.
+    configOptions.diagnosticSettings.reportDuplicateImport = 'error';
+    analysisResults = TestUtils.typeAnalyzeSampleFiles(['duplicateImports1.py'], configOptions);
+    validateResults(analysisResults, 2);
+});

server/src/tests/samples/duplicateImports1.py

@@ -0,0 +1,15 @@
+# This sample tests the duplicate import detection.
+
+import sys
+
+# This should generate an error because Any is duplicated
+from typing import Any, Dict, Any
+
+# This should generate an error because sys is duplicated
+import sys
+
+
+a: Dict[Any, Any]
+b = sys.api_version
+
+