diff --git a/docs/configuration.md b/docs/configuration.md index 0abc6f834..e412cf477 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -102,6 +102,8 @@ The following settings control pyright’s diagnostic output (warnings or errors **reportPrivateUsage** [boolean or string, optional]: Generate or suppress diagnostics for incorrect usage of private or protected variables or functions. Protected class members begin with a single underscore (“_”) and can be accessed only by subclasses. Private class members begin with a double underscore but do not end in a double underscore and can be accessed only within the declaring class. Variables and functions declared outside of a class are considered private if their names start with either a single or double underscore, and they cannot be accessed outside of the declaring module. The default value for this setting is 'none'. +**reportPrivateImportUsage** [boolean or string, optional]: Generate or suppress diagnostics for use of a symbol from a "py.typed" module that is not meant to be exported from that module. The default value for this setting is 'error'. + **reportConstantRedefinition** [boolean or string, optional]: Generate or suppress diagnostics for attempts to redefine variables whose names are all-caps with underscores and numerals. The default value for this setting is 'none'. **reportIncompatibleMethodOverride** [boolean or string, optional]: Generate or suppress diagnostics for methods that override a method of the same name in a base class in an incompatible manner (wrong number of parameters, incompatible parameter types, or incompatible return type). The default value for this setting is 'none'. @@ -294,6 +296,7 @@ The following table lists the default severity levels for each diagnostic rule w | reportUntypedBaseClass | "none" | "none" | "error" | | reportUntypedNamedTuple | "none" | "none" | "error" | | reportPrivateUsage | "none" | "none" | "error" | +| reportPrivateImportUsage | "none" | "error" | "error" | | reportConstantRedefinition | "none" | "none" | "error" | | reportIncompatibleMethodOverride | "none" | "none" | "error" | | reportIncompatibleVariableOverride | "none" | "none" | "error" | diff --git a/packages/pyright-internal/src/analyzer/aliasDeclarationUtils.ts b/packages/pyright-internal/src/analyzer/aliasDeclarationUtils.ts index f9d9e4cae..ea7064b46 100644 --- a/packages/pyright-internal/src/analyzer/aliasDeclarationUtils.ts +++ b/packages/pyright-internal/src/analyzer/aliasDeclarationUtils.ts @@ -13,6 +13,8 @@ import { Symbol } from './symbol'; export interface ResolvedAliasInfo { declaration: Declaration; isPrivate: boolean; + privatePyTypedImported?: string; + privatePyTypedImporter?: string; } // If the specified declaration is an alias declaration that points to a symbol, @@ -28,12 +30,17 @@ export function resolveAliasDeclaration( let curDeclaration: Declaration | undefined = declaration; const alreadyVisited: Declaration[] = []; let isPrivate = false; + let isPrivatePyTypedImport = false; + let privatePyTypedImported: string | undefined; + let privatePyTypedImporter: string | undefined; while (true) { if (curDeclaration.type !== DeclarationType.Alias || !curDeclaration.symbolName) { return { declaration: curDeclaration, isPrivate, + privatePyTypedImported, + privatePyTypedImporter, }; } @@ -43,6 +50,8 @@ export function resolveAliasDeclaration( return { declaration: curDeclaration, isPrivate, + privatePyTypedImported, + privatePyTypedImporter, }; } @@ -84,6 +93,18 @@ export function resolveAliasDeclaration( // we use all of the overloads if it's an overloaded function. curDeclaration = declarations[declarations.length - 1]; + if (isPrivatePyTypedImport) { + privatePyTypedImported = privatePyTypedImported ?? curDeclaration?.moduleName; + } + + if (symbol.isPrivatePyTypedImport()) { + isPrivatePyTypedImport = true; + } + + if (isPrivatePyTypedImport) { + privatePyTypedImporter = privatePyTypedImporter ?? curDeclaration?.moduleName; + } + // Make sure we don't follow a circular list indefinitely. if (alreadyVisited.find((decl) => decl === curDeclaration)) { // If the path path of the alias points back to the original path, use the submodule @@ -101,6 +122,8 @@ export function resolveAliasDeclaration( return { declaration, isPrivate, + privatePyTypedImported, + privatePyTypedImporter, }; } alreadyVisited.push(curDeclaration); diff --git a/packages/pyright-internal/src/analyzer/typeEvaluator.ts b/packages/pyright-internal/src/analyzer/typeEvaluator.ts index b7c0f0d0e..5aa8b58b9 100644 --- a/packages/pyright-internal/src/analyzer/typeEvaluator.ts +++ b/packages/pyright-internal/src/analyzer/typeEvaluator.ts @@ -4829,6 +4829,18 @@ export function createTypeEvaluator( node.memberName ); } + + if (symbol.isPrivatePyTypedImport()) { + addDiagnostic( + getFileInfo(node).diagnosticRuleSet.reportPrivateImportUsage, + DiagnosticRule.reportPrivateImportUsage, + Localizer.Diagnostic.privateImportFromPyTypedModule().format({ + name: memberName, + module: baseType.moduleName, + }), + node.memberName + ); + } } else { // Does the module export a top-level __getattr__ function? if (usage.method === 'get') { @@ -16724,15 +16736,37 @@ export function createTypeEvaluator( return undefined; } - if (node.nodeType === ParseNodeType.ImportFromAs && resolvedAliasInfo.isPrivate) { - addDiagnostic( - getFileInfo(node).diagnosticRuleSet.reportPrivateUsage, - DiagnosticRule.reportPrivateUsage, - Localizer.Diagnostic.privateUsedOutsideOfModule().format({ - name: node.name.value, - }), - node.name - ); + if (node.nodeType === ParseNodeType.ImportFromAs) { + if (resolvedAliasInfo.isPrivate) { + addDiagnostic( + getFileInfo(node).diagnosticRuleSet.reportPrivateUsage, + DiagnosticRule.reportPrivateUsage, + Localizer.Diagnostic.privateUsedOutsideOfModule().format({ + name: node.name.value, + }), + node.name + ); + } + + if (resolvedAliasInfo.privatePyTypedImporter) { + const diag = new DiagnosticAddendum(); + if (resolvedAliasInfo.privatePyTypedImported) { + diag.addMessage( + Localizer.DiagnosticAddendum.privateImportFromPyTypedSource().format({ + module: resolvedAliasInfo.privatePyTypedImported, + }) + ); + } + addDiagnostic( + getFileInfo(node).diagnosticRuleSet.reportPrivateImportUsage, + DiagnosticRule.reportPrivateImportUsage, + Localizer.Diagnostic.privateImportFromPyTypedModule().format({ + name: node.name.value, + module: resolvedAliasInfo.privatePyTypedImporter, + }) + diag.getString(), + node.name + ); + } } return getInferredTypeOfDeclaration(aliasDecl); diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index 9c0832ae7..8b4f4349a 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -172,6 +172,10 @@ export interface DiagnosticRuleSet { // the owning class or module? reportPrivateUsage: DiagnosticLevel; + // Report usage of an import from a py.typed module that is + // not meant to be re-exported from that module. + reportPrivateImportUsage: DiagnosticLevel; + // Report attempts to redefine variables that are in all-caps. reportConstantRedefinition: DiagnosticLevel; @@ -311,6 +315,7 @@ export function getDiagLevelDiagnosticRules() { DiagnosticRule.reportUntypedBaseClass, DiagnosticRule.reportUntypedNamedTuple, DiagnosticRule.reportPrivateUsage, + DiagnosticRule.reportPrivateImportUsage, DiagnosticRule.reportConstantRedefinition, DiagnosticRule.reportIncompatibleMethodOverride, DiagnosticRule.reportIncompatibleVariableOverride, @@ -384,6 +389,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet { reportUntypedBaseClass: 'none', reportUntypedNamedTuple: 'none', reportPrivateUsage: 'none', + reportPrivateImportUsage: 'none', reportConstantRedefinition: 'none', reportIncompatibleMethodOverride: 'none', reportIncompatibleVariableOverride: 'none', @@ -453,6 +459,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet { reportUntypedBaseClass: 'none', reportUntypedNamedTuple: 'none', reportPrivateUsage: 'none', + reportPrivateImportUsage: 'error', reportConstantRedefinition: 'none', reportIncompatibleMethodOverride: 'none', reportIncompatibleVariableOverride: 'none', @@ -522,6 +529,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet { reportUntypedBaseClass: 'error', reportUntypedNamedTuple: 'error', reportPrivateUsage: 'error', + reportPrivateImportUsage: 'error', reportConstantRedefinition: 'error', reportIncompatibleMethodOverride: 'error', reportIncompatibleVariableOverride: 'error', @@ -1053,6 +1061,13 @@ export class ConfigOptions { defaultSettings.reportPrivateUsage ), + // Read the "reportPrivateImportUsage" entry. + reportPrivateImportUsage: this._convertDiagnosticLevel( + configObj.reportPrivateImportUsage, + DiagnosticRule.reportPrivateImportUsage, + defaultSettings.reportPrivateImportUsage + ), + // Read the "reportConstantRedefinition" entry. reportConstantRedefinition: this._convertDiagnosticLevel( configObj.reportConstantRedefinition, diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index 3958dc30e..98a56da85 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -42,6 +42,7 @@ export enum DiagnosticRule { reportUntypedBaseClass = 'reportUntypedBaseClass', reportUntypedNamedTuple = 'reportUntypedNamedTuple', reportPrivateUsage = 'reportPrivateUsage', + reportPrivateImportUsage = 'reportPrivateImportUsage', reportConstantRedefinition = 'reportConstantRedefinition', reportIncompatibleMethodOverride = 'reportIncompatibleMethodOverride', reportIncompatibleVariableOverride = 'reportIncompatibleVariableOverride', diff --git a/packages/pyright-internal/src/languageService/completionProvider.ts b/packages/pyright-internal/src/languageService/completionProvider.ts index 8ba0849bc..f8240524f 100644 --- a/packages/pyright-internal/src/languageService/completionProvider.ts +++ b/packages/pyright-internal/src/languageService/completionProvider.ts @@ -1992,6 +1992,12 @@ export class CompletionProvider { completionList: CompletionList, detail: SymbolDetail ) { + // If the symbol is a py.typed import that is not supposed to be re-exported, + // don't offer it as a completion suggestion. + if (symbol.isPrivatePyTypedImport()) { + return; + } + let primaryDecl = getLastTypedDeclaredForSymbol(symbol); if (!primaryDecl) { const declarations = symbol.getDeclarations(); diff --git a/packages/pyright-internal/src/localization/localize.ts b/packages/pyright-internal/src/localization/localize.ts index 3fa82504c..c89f2d062 100644 --- a/packages/pyright-internal/src/localization/localize.ts +++ b/packages/pyright-internal/src/localization/localize.ts @@ -565,6 +565,10 @@ export namespace Localizer { export const parenthesizedContextManagerIllegal = () => getRawString('Diagnostic.parenthesizedContextManagerIllegal'); export const positionArgAfterNamedArg = () => getRawString('Diagnostic.positionArgAfterNamedArg'); + export const privateImportFromPyTypedModule = () => + new ParameterizedString<{ name: string; module: string }>( + getRawString('Diagnostic.privateImportFromPyTypedModule') + ); export const positionOnlyAfterArgs = () => getRawString('Diagnostic.positionOnlyAfterArgs'); export const positionOnlyAfterKeywordOnly = () => getRawString('Diagnostic.positionOnlyAfterKeywordOnly'); export const positionOnlyIncompatible = () => getRawString('Diagnostic.positionOnlyIncompatible'); @@ -947,6 +951,10 @@ export namespace Localizer { export const paramSpecOverload = () => getRawString('DiagnosticAddendum.paramSpecOverload'); export const paramType = () => new ParameterizedString<{ paramType: string }>(getRawString('DiagnosticAddendum.paramType')); + export const privateImportFromPyTypedSource = () => + new ParameterizedString<{ module: string }>( + getRawString('DiagnosticAddendum.privateImportFromPyTypedSource') + ); export const propertyMethodIncompatible = () => new ParameterizedString<{ name: string }>(getRawString('DiagnosticAddendum.propertyMethodIncompatible')); export const propertyMethodMissing = () => diff --git a/packages/pyright-internal/src/localization/package.nls.en-us.json b/packages/pyright-internal/src/localization/package.nls.en-us.json index e1b381dc2..a3ec94445 100644 --- a/packages/pyright-internal/src/localization/package.nls.en-us.json +++ b/packages/pyright-internal/src/localization/package.nls.en-us.json @@ -272,6 +272,7 @@ "paramTypePartiallyUnknown": "Type of parameter \"{paramName}\" is partially unknown", "parenthesizedContextManagerIllegal": "Parentheses within \"with\" statement requires Python 3.9 or newer", "positionArgAfterNamedArg": "Positional argument cannot appear after keyword arguments", + "privateImportFromPyTypedModule": "\"{name}\" is not exported from module \"{module}\"", "positionOnlyAfterArgs": "Position-only argument separator not allowed after \"*\" parameter", "positionOnlyAfterKeywordOnly": "\"/\" parameter must appear before \"*\" parameter", "positionOnlyIncompatible": "Position-only argument separator requires Python 3.8 or newer", @@ -486,6 +487,7 @@ "paramAssignment": "Parameter {index}: type \"{sourceType}\" cannot be assigned to type \"{destType}\"", "paramSpecOverload": "ParamSpec cannot be used with overloaded function", "paramType": "Parameter type is \"{paramType}\"", + "privateImportFromPyTypedSource": "Import from \"{module}\" instead", "propertyMethodIncompatible": "Property method \"{name}\" is incompatible", "propertyMethodMissing": "Property method \"{name}\" is missing in override", "propertyMissingDeleter": "Property \"{name}\" has no defined deleter", diff --git a/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.pkg-vs-module1.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.pkg-vs-module1.fourslash.ts index abad97097..626369a7b 100644 --- a/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.pkg-vs-module1.fourslash.ts +++ b/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.pkg-vs-module1.fourslash.ts @@ -11,7 +11,7 @@ //// return True // @filename: typings/package1/__init__.pyi -//// from .subpackage import func1 +//// from .subpackage import func1 as func1 // @filename: typings/package1/subpackage/__init__.pyi //// def func1() -> bool: ... diff --git a/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.pkg-vs-module2.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.pkg-vs-module2.fourslash.ts index 53b3fce18..168422c83 100644 --- a/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.pkg-vs-module2.fourslash.ts +++ b/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.pkg-vs-module2.fourslash.ts @@ -11,7 +11,7 @@ //// return True // @filename: typings/package1/__init__.pyi -//// from .subpackage import func1 +//// from .subpackage import func1 as func1 // @filename: typings/package1/subpackage.pyi //// def func1() -> bool: ... diff --git a/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.stubs-package.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.stubs-package.fourslash.ts index 224b2be33..612e21280 100644 --- a/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.stubs-package.fourslash.ts +++ b/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.stubs-package.fourslash.ts @@ -2,7 +2,7 @@ // @filename: package1-stubs/__init__.pyi // @library: true -//// from .api import func1 +//// from .api import func1 as func1 // @filename: package1-stubs/api.pyi // @library: true @@ -10,7 +10,7 @@ // @filename: package1/__init__.py // @library: true -//// from .api import func1 +//// from .api import func1 as func1 // @filename: package1/api.py // @library: true diff --git a/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.typeshed.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.typeshed.fourslash.ts index 962a9b845..3ec1dae8e 100644 --- a/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.typeshed.fourslash.ts +++ b/packages/pyright-internal/src/tests/fourslash/hover.docFromSrc.typeshed.fourslash.ts @@ -2,7 +2,7 @@ // @filename: requests/__init__.pyi // @library: true -//// from .api import head +//// from .api import head as head // @filename: requests/api.pyi // @library: true diff --git a/packages/pyright-internal/src/tests/fourslash/import.pytyped.privateSymbols.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/import.pytyped.privateSymbols.fourslash.ts new file mode 100644 index 000000000..b9d39f813 --- /dev/null +++ b/packages/pyright-internal/src/tests/fourslash/import.pytyped.privateSymbols.fourslash.ts @@ -0,0 +1,65 @@ +/// + +// @filename: pyrightconfig.json +//// { +//// "typeCheckingMode": "basic" +//// } + +// @filename: testLib/py.typed +// @library: true +//// + +// @filename: testLib/__init__.py +// @library: true +//// from .module1 import one as one, two, three +//// four: int = two * two +//// _five: int = two + three +//// _six: int = 6 +//// __all__ = ["_six"] + +// @filename: testLib/module1.py +// @library: true +//// one: int = 1 +//// two: int = 2 +//// three: int = 3 + +// @filename: .src/test1.py +//// # pyright: reportPrivateUsage=true, reportPrivateImportUsage=true +//// from testLib import one +//// from testLib import [|/*marker1*/two|] as two_alias +//// from testLib import [|/*marker2*/three|] +//// from testLib import four +//// from testLib import [|/*marker3*/_five|] +//// from testLib import _six +//// import testLib +//// testLib.one +//// testLib.[|/*marker4*/two|] +//// testLib.[|/*marker5*/three|] +//// testLib.four +//// testLib.[|/*marker6*/_five|] +//// testLib._six + +// @ts-ignore +await helper.verifyDiagnostics({ + marker1: { + category: 'error', + message: `"two" is not exported from module "testLib"\n  Import from \"testLib.module1\" instead`, + }, + marker2: { + category: 'error', + message: `"three" is not exported from module "testLib"\n  Import from \"testLib.module1\" instead`, + }, + marker3: { + category: 'error', + message: `"_five" is private and used outside of the module in which it is declared`, + }, + marker4: { category: 'error', message: `"two" is not exported from module "testLib"` }, + marker5: { + category: 'error', + message: `"three" is not exported from module "testLib"`, + }, + marker6: { + category: 'error', + message: `"_five" is private and used outside of the module in which it is declared`, + }, +}); diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index efa8794f7..c3e1cc7f2 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -388,6 +388,17 @@ "error" ] }, + "reportPrivateImportUsage": { + "type": "string", + "description": "Diagnostics for incorrect usage of symbol imported from a \"py.typed\" module that is not re-exported from that module.", + "default": "error", + "enum": [ + "none", + "information", + "warning", + "error" + ] + }, "reportConstantRedefinition": { "type": "string", "description": "Diagnostics for attempts to redefine variables whose names are all-caps with underscores and numerals.", diff --git a/packages/vscode-pyright/schemas/pyrightconfig.schema.json b/packages/vscode-pyright/schemas/pyrightconfig.schema.json index 917d69642..26224b9d1 100644 --- a/packages/vscode-pyright/schemas/pyrightconfig.schema.json +++ b/packages/vscode-pyright/schemas/pyrightconfig.schema.json @@ -281,6 +281,12 @@ "title": "Controls reporting of private variables and functions used outside of the owning class or module and usage of protected members outside of subclasses", "default": "none" }, + "reportPrivateImportUsage": { + "$id": "#/properties/reportPrivateImportUsage", + "$ref": "#/definitions/diagnostic", + "title": "Controls reporting of improper usage of symbol imported from a \"py.typed\" module that is not re-exported from that module", + "default": "error" + }, "reportConstantRedefinition": { "$id": "#/properties/reportConstantRedefinition", "$ref": "#/definitions/diagnostic",