Added reportPrivateImportUsage diagnostic rule, which reports usage of a symbol from a py.typed library that is not intended to be re-exported by the library's author. The rule is on by default in basic type checking mode but can be disabled. Completion provider no longer offers these symbols as completion suggestions.

This commit is contained in:
Eric Traut 2021-09-12 01:10:57 -07:00
parent b4430590b5
commit 9035f64c71
15 changed files with 188 additions and 14 deletions

View File

@ -102,6 +102,8 @@ The following settings control pyrights 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" |

View File

@ -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);

View File

@ -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);

View File

@ -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,

View File

@ -42,6 +42,7 @@ export enum DiagnosticRule {
reportUntypedBaseClass = 'reportUntypedBaseClass',
reportUntypedNamedTuple = 'reportUntypedNamedTuple',
reportPrivateUsage = 'reportPrivateUsage',
reportPrivateImportUsage = 'reportPrivateImportUsage',
reportConstantRedefinition = 'reportConstantRedefinition',
reportIncompatibleMethodOverride = 'reportIncompatibleMethodOverride',
reportIncompatibleVariableOverride = 'reportIncompatibleVariableOverride',

View File

@ -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();

View File

@ -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 = () =>

View File

@ -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",

View File

@ -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: ...

View File

@ -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: ...

View File

@ -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

View File

@ -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

View File

@ -0,0 +1,65 @@
/// <reference path="fourslash.ts" />
// @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`,
},
});

View File

@ -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.",

View File

@ -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",