Reverted a recent change that extended the reportPrivateImportUsage check to enforce imports from submodules whose names begin with an underscore. This change was disruptive to users of the popular pandas library because it exports a submodule named _typing. This was originally intended to be experimental or private, but it was never fully addressed. The pandas maintainers will work to fix this issue (pandas-dev/pandas#55231) over the next year. I'm going to back out this change to pyright in the meantime.

Author: erictraut

packages/pyright-internal/src/analyzer/analyzerFileInfo.ts

@@ -61,7 +61,6 @@ export interface AnalyzerFileInfo {
     isTypeshedStubFile: boolean;
     isBuiltInStubFile: boolean;
     isInPyTypedPackage: boolean;
-    isModulePrivate: boolean;
     ipythonMode: IPythonMode;
     accessedSymbolSet: Set<number>;
 }

packages/pyright-internal/src/analyzer/binder.ts

@@ -272,7 +272,7 @@ export class Binder extends ParseTreeWalker {
 
         this._addTypingImportAliasesFromBuiltinsScope();
 
-        const moduleScope = this._createNewScope(
+        this._createNewScope(
             isBuiltInModule ? ScopeType.Builtin : ScopeType.Module,
             this._fileInfo.builtinsScope,
             /* proxyScope */ undefined,
@@ -310,24 +310,17 @@ export class Binder extends ParseTreeWalker {
         // Perform all analysis that was deferred during the first pass.
         this._bindDeferred();
 
-        if (this._fileInfo.isModulePrivate) {
-            // All symbols in a private module are private outside of the package.
-            moduleScope.symbolTable.forEach((symbol) => {
-                symbol.setPrivatePyTypedImport();
-            });
-        } else {
-            // Use the __all__ list to determine whether any potential private
-            // symbols should be made externally hidden or private.
-            this._potentialHiddenSymbols.forEach((symbol, name) => {
-                if (!this._dunderAllNames?.some((sym) => sym === name)) {
-                    if (this._fileInfo.isStubFile) {
-                        symbol.setIsExternallyHidden();
-                    } else {
-                        symbol.setPrivatePyTypedImport();
-                    }
+        // Use the __all__ list to determine whether any potential private
+        // symbols should be made externally hidden or private.
+        this._potentialHiddenSymbols.forEach((symbol, name) => {
+            if (!this._dunderAllNames?.some((sym) => sym === name)) {
+                if (this._fileInfo.isStubFile) {
+                    symbol.setIsExternallyHidden();
+                } else {
+                    symbol.setPrivatePyTypedImport();
                 }
-            });
-        }
+            }
+        });
 
         this._potentialPrivateSymbols.forEach((symbol, name) => {
             if (!this._dunderAllNames?.some((sym) => sym === name)) {

packages/pyright-internal/src/analyzer/importResolver.ts

@@ -46,7 +46,6 @@ export interface ModuleNameAndType {
 export interface ModuleImportInfo extends ModuleNameAndType {
     isTypeshedFile: boolean;
     isThirdPartyPyTypedPresent: boolean;
-    isModulePrivate: boolean;
 }
 
 export interface ModuleNameInfoFromPath {
@@ -936,7 +935,6 @@ export class ImportResolver {
             importType: ImportType.Local,
             isStubFile: false,
             isNativeLib: false,
-            isModulePrivate: false,
             implicitImports: new Map<string, ImplicitImport>(),
             filteredImplicitImports: new Map<string, ImplicitImport>(),
             nonStubImportResult: undefined,
@@ -1120,7 +1118,6 @@ export class ImportResolver {
         let importType = ImportType.BuiltIn;
         let isLocalTypingsFile = false;
         let isThirdPartyPyTypedPresent = false;
-        const isModulePrivate = false;
         let isTypeshedFile = false;
 
         const importFailureInfo: string[] = [];
@@ -1163,7 +1160,6 @@ export class ImportResolver {
                         isTypeshedFile: true,
                         isLocalTypingsFile,
                         isThirdPartyPyTypedPresent,
-                        isModulePrivate,
                     };
                 }
             }
@@ -1300,14 +1296,7 @@ export class ImportResolver {
         }
 
         if (moduleName) {
-            return {
-                moduleName,
-                importType,
-                isTypeshedFile,
-                isLocalTypingsFile,
-                isThirdPartyPyTypedPresent,
-                isModulePrivate,
-            };
+            return { moduleName, importType, isTypeshedFile, isLocalTypingsFile, isThirdPartyPyTypedPresent };
         }
 
         if (allowInvalidModuleName && moduleNameWithInvalidCharacters) {
@@ -1317,7 +1306,6 @@ export class ImportResolver {
                 importType,
                 isLocalTypingsFile,
                 isThirdPartyPyTypedPresent,
-                isModulePrivate,
             };
         }
 
@@ -1328,7 +1316,6 @@ export class ImportResolver {
             importType: ImportType.Local,
             isLocalTypingsFile,
             isThirdPartyPyTypedPresent,
-            isModulePrivate,
         };
     }
 
@@ -1368,7 +1355,6 @@ export class ImportResolver {
         let implicitImports = new Map<string, ImplicitImport>();
         let packageDirectory: Uri | undefined;
         let pyTypedInfo: PyTypedInfo | undefined;
-        let isModulePrivate = false;
 
         // Handle the "from . import XXX" case.
         if (moduleDescriptor.nameParts.length === 0) {
@@ -1393,12 +1379,7 @@ export class ImportResolver {
             for (let i = 0; i < moduleDescriptor.nameParts.length; i++) {
                 const isFirstPart = i === 0;
                 const isLastPart = i === moduleDescriptor.nameParts.length - 1;
-                const namePart = moduleDescriptor.nameParts[i];
-                dirPath = dirPath.combinePaths(namePart);
-
-                if (SymbolNameUtils.isProtectedName(namePart)) {
-                    isModulePrivate = true;
-                }
+                dirPath = dirPath.combinePaths(moduleDescriptor.nameParts[i]);
 
                 if (useStubPackage && isFirstPart) {
                     dirPath = dirPath.addPath(stubsSuffix);
@@ -1514,12 +1495,6 @@ export class ImportResolver {
             importFound = resolvedPaths.length >= moduleDescriptor.nameParts.length;
         }
 
-        // Modules are considered private only if they are stub files or located
-        // within a py.typed package.
-        if (!isStubFile && !pyTypedInfo) {
-            isModulePrivate = false;
-        }
-
         return {
             importName,
             isRelative: false,
@@ -1536,7 +1511,6 @@ export class ImportResolver {
             isNativeLib,
             implicitImports,
             pyTypedInfo,
-            isModulePrivate,
             filteredImplicitImports: implicitImports,
             packageDirectory,
         };

packages/pyright-internal/src/analyzer/importResult.ts

@@ -99,10 +99,6 @@ export interface ImportResult {
     // from .py here.
     nonStubImportResult?: ImportResult | undefined;
 
-    // Are the target module or its containing directories named in a
-    // manner that indicates the module should be treated as private?
-    isModulePrivate: boolean;
-
     // Is there a "py.typed" file (as described in PEP 561) present in
     // the package that was used to resolve the import?
     pyTypedInfo?: PyTypedInfo | undefined;

packages/pyright-internal/src/analyzer/program.ts

@@ -73,7 +73,6 @@ interface UpdateImportInfo {
     isTypeshedFile: boolean;
     isThirdPartyImport: boolean;
     isPyTypedPresent: boolean;
-    isModulePrivate: boolean;
 }
 
 export type PreCheckCallback = (parserOutput: ParserOutput, evaluator: TypeEvaluator) => void;
@@ -348,7 +347,6 @@ export class Program {
             importName,
             isThirdPartyImport,
             isInPyTypedPackage,
-            /* isModulePrivate */ false,
             this._editModeTracker,
             this._console,
             this._logTracker
@@ -377,7 +375,6 @@ export class Program {
                 moduleImportInfo.moduleName,
                 /* isThirdPartyImport */ false,
                 moduleImportInfo.isThirdPartyPyTypedPresent,
-                moduleImportInfo.isModulePrivate,
                 this._editModeTracker,
                 this._console,
                 this._logTracker,
@@ -1365,30 +1362,22 @@ export class Program {
         const getThirdPartyImportInfo = (importResult: ImportResult) => {
             let isThirdPartyImport = false;
             let isPyTypedPresent = false;
-            let isModulePrivate = false;
 
             if (importResult.importType === ImportType.ThirdParty) {
                 isThirdPartyImport = true;
                 if (importResult.pyTypedInfo) {
                     isPyTypedPresent = true;
                 }
-                if (importResult.isModulePrivate) {
-                    isModulePrivate = true;
-                }
             } else if (sourceFileInfo.isThirdPartyImport && importResult.importType === ImportType.Local) {
                 isThirdPartyImport = true;
                 if (sourceFileInfo.isThirdPartyPyTypedPresent) {
                     isPyTypedPresent = true;
                 }
-                if (importResult.isModulePrivate) {
-                    isModulePrivate = true;
-                }
             }
 
             return {
                 isThirdPartyImport,
                 isPyTypedPresent,
-                isModulePrivate,
             };
         };
 
@@ -1406,7 +1395,6 @@ export class Program {
                     isTypeshedFile: false,
                     isThirdPartyImport: false,
                     isPyTypedPresent: false,
-                    isModulePrivate: false,
                 });
             }
         }
@@ -1424,7 +1412,6 @@ export class Program {
                                     !!importResult.isStdlibTypeshedFile || !!importResult.isThirdPartyTypeshedFile,
                                 isThirdPartyImport: thirdPartyTypeInfo.isThirdPartyImport,
                                 isPyTypedPresent: thirdPartyTypeInfo.isPyTypedPresent,
-                                isModulePrivate: thirdPartyTypeInfo.isModulePrivate,
                             });
                         }
                     }
@@ -1440,7 +1427,6 @@ export class Program {
                                     !!importResult.isStdlibTypeshedFile || !!importResult.isThirdPartyTypeshedFile,
                                 isThirdPartyImport: thirdPartyTypeInfo.isThirdPartyImport,
                                 isPyTypedPresent: thirdPartyTypeInfo.isPyTypedPresent,
-                                isModulePrivate: thirdPartyTypeInfo.isModulePrivate,
                             });
                         }
                     }
@@ -1510,7 +1496,6 @@ export class Program {
                         moduleImportInfo.moduleName,
                         importInfo.isThirdPartyImport,
                         importInfo.isPyTypedPresent,
-                        importInfo.isModulePrivate,
                         this._editModeTracker,
                         this._console,
                         this._logTracker
@@ -1627,7 +1612,6 @@ export class Program {
             moduleImportInfo.moduleName,
             /* isThirdPartyImport */ false,
             /* isInPyTypedPackage */ false,
-            /* isModulePrivate */ false,
             this._editModeTracker,
             this._console,
             this._logTracker

packages/pyright-internal/src/analyzer/programTypes.ts

@@ -9,7 +9,7 @@ import { ConsoleInterface } from '../common/console';
 import { LogTracker } from '../common/logTracker';
 import { ServiceProvider } from '../common/serviceProvider';
 import { Uri } from '../common/uri/uri';
-import { IPythonMode, SourceFile, SourceFileEditMode } from './sourceFile';
+import { SourceFileEditMode, IPythonMode, SourceFile } from './sourceFile';
 
 export interface ISourceFileFactory {
     createSourceFile(
@@ -18,7 +18,6 @@ export interface ISourceFileFactory {
         moduleName: string,
         isThirdPartyImport: boolean,
         isThirdPartyPyTypedPresent: boolean,
-        isModulePrivate: boolean,
         editMode: SourceFileEditMode,
         console?: ConsoleInterface,
         logTracker?: LogTracker,

packages/pyright-internal/src/analyzer/sourceFile.ts

@@ -232,12 +232,6 @@ export class SourceFile {
     // "py.typed" file.
     private readonly _isThirdPartyPyTypedPresent: boolean;
 
-    // Indicates that the file is a submodule within a stub
-    // package or a py.typed package and is named such that it
-    // is "private" or contained within a subdirectory that
-    // is named such that it is "private".
-    private readonly _isModulePrivate: boolean;
-
     private readonly _editMode: SourceFileEditMode;
 
     // Settings that control which diagnostics should be output. The rules
@@ -261,7 +255,6 @@ export class SourceFile {
         moduleName: string,
         isThirdPartyImport: boolean,
         isThirdPartyPyTypedPresent: boolean,
-        isModulePrivate: boolean,
         editMode: SourceFileEditMode,
         console?: ConsoleInterface,
         logTracker?: LogTracker,
@@ -278,7 +271,6 @@ export class SourceFile {
         this._isStubFile = uri.hasExtension('.pyi');
         this._isThirdPartyImport = isThirdPartyImport;
         this._isThirdPartyPyTypedPresent = isThirdPartyPyTypedPresent;
-        this._isModulePrivate = isModulePrivate;
         const fileName = uri.fileName;
         this._isTypingStubFile =
             this._isStubFile && (this._uri.pathEndsWith('stdlib/typing.pyi') || fileName === 'typing_extensions.pyi');
@@ -1353,7 +1345,6 @@ export class SourceFile {
             isTypeshedStubFile: this._isTypeshedStubFile,
             isBuiltInStubFile: this._isBuiltInStubFile,
             isInPyTypedPackage: this._isThirdPartyPyTypedPresent,
-            isModulePrivate: this._isModulePrivate,
             ipythonMode: this._ipythonMode,
             accessedSymbolSet: new Set<number>(),
         };

packages/pyright-internal/src/common/serviceProviderExtensions.ts

@@ -118,7 +118,6 @@ const DefaultSourceFileFactory: ISourceFileFactory = {
         moduleName: string,
         isThirdPartyImport: boolean,
         isThirdPartyPyTypedPresent: boolean,
-        isModulePrivate: boolean,
         editMode: SourceFileEditMode,
         console?: ConsoleInterface,
         logTracker?: LogTracker,
@@ -130,7 +129,6 @@ const DefaultSourceFileFactory: ISourceFileFactory = {
             moduleName,
             isThirdPartyImport,
             isThirdPartyPyTypedPresent,
-            isModulePrivate,
             editMode,
             console,
             logTracker,

packages/pyright-internal/src/tests/fourslash/import.pytyped.privateSymbols.fourslash.ts

@@ -12,7 +12,6 @@
 // @filename: testLib/__init__.py
 // @library: true
 //// from .module1 import one as one, two, three
-//// from ._module2 import ten as ten
 //// four: int = two * two
 //// _five: int = two + three
 //// _six: int = 6
@@ -24,10 +23,6 @@
 //// two: int = 2
 //// three: int = 3
 
-// @filename: testLib/_module2/__init__.py
-// @library: true
-//// ten: int = 10
-
 // @filename: .src/test1.py
 //// # pyright: reportPrivateUsage=true, reportPrivateImportUsage=true
 //// from testLib import one
@@ -43,12 +38,6 @@
 //// testLib.four
 //// testLib.[|/*marker6*/_five|]
 //// testLib._six
-////
-//// from testLib._module2 import [|/*marker7*/ten|]
-//// from testLib import ten
-//// import testLib._module2
-//// testLib.ten
-//// testLib._module2.[|/*marker8*/ten|]
 
 // @ts-ignore
 await helper.verifyDiagnostics({
@@ -73,12 +62,4 @@ await helper.verifyDiagnostics({
         category: 'error',
         message: `"_five" is private and used outside of the module in which it is declared`,
     },
-    marker7: {
-        category: 'error',
-        message: `"ten" is not exported from module "testLib._module2"`,
-    },
-    marker8: {
-        category: 'error',
-        message: `"ten" is not exported from module "testLib._module2"`,
-    },
 });