Skip to content

Commit

Permalink
add rule to detect private non-py.typed import usages
Browse files Browse the repository at this point in the history
  • Loading branch information
@DetachHead
DetachHead committed Mar 24, 2024
1 parent 6976347 commit 5cb08f6
Show file tree
Hide file tree
Showing 9 changed files with 199 additions and 117 deletions.
185 changes: 95 additions & 90 deletions docs/configuration.md
View file

Large diffs are not rendered by default.

31 changes: 16 additions & 15 deletions packages/pyright-internal/src/analyzer/binder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,10 @@ export class Binder extends ParseTreeWalker {
if (!this._dunderAllNames?.some((sym) => sym === name)) {
if (this._fileInfo.isStubFile) {
symbol.setIsExternallyHidden();
} else {
} else if (this._fileInfo.isInPyTypedPackage) {
symbol.setPrivatePyTypedImport();
} else {
symbol.setPrivateNonPyTypedImport();
}
}
});
Expand Down Expand Up @@ -1749,13 +1751,14 @@ export class Binder extends ParseTreeWalker {
node.module.nameParts.length !== 1 ||
node.module.nameParts[0].value !== node.alias.value)
) {
if (this._fileInfo.isStubFile || this._fileInfo.isInPyTypedPackage) {
// PEP 484 indicates that imported symbols should not be
// considered "reexported" from a type stub file unless
// they are imported using the "as" form and the aliased
// name is entirely redundant.
this._potentialHiddenSymbols.set(symbolName, symbol);
}
// PEP 484 indicates that imported symbols should not be
// considered "reexported" from a type stub file unless
// they are imported using the "as" form and the aliased
// name is entirely redundant.

// in basedpyright, we don't care whether it's a stub file or not. this should
// apply to regular modules too. this is also consistent with mypy'ss behavior.
this._potentialHiddenSymbols.set(symbolName, symbol);
}

const importInfo = AnalyzerNodeInfo.getImportInfo(node.module);
Expand Down Expand Up @@ -1946,13 +1949,11 @@ export class Binder extends ParseTreeWalker {
!importSymbolNode.alias ||
importSymbolNode.alias.value !== importSymbolNode.name.value
) {
if (this._fileInfo.isStubFile || this._fileInfo.isInPyTypedPackage) {
// PEP 484 indicates that imported symbols should not be
// considered "reexported" from a type stub file unless
// they are imported using the "as" form using a redundant form.
// Py.typed packages follow the same rule as PEP 484.
this._potentialHiddenSymbols.set(nameNode.value, symbol);
}
// PEP 484 indicates that imported symbols should not be
// considered "reexported" from a type stub file unless
// they are imported using the "as" form using a redundant form.
// Py.typed packages follow the same rule as PEP 484.
this._potentialHiddenSymbols.set(nameNode.value, symbol);
}
}
}
Expand Down
34 changes: 28 additions & 6 deletions packages/pyright-internal/src/analyzer/declarationUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ import { Symbol } from './symbol';
export interface ResolvedAliasInfo {
declaration: Declaration | undefined;
isPrivate: boolean;
privatePyTypedImported?: string;
privateImported?: string;
privatePyTypedImporter?: string;
privateNonPyTypedImporter?: string;
}

export function hasTypeForDeclaration(declaration: Declaration): boolean {
Expand Down Expand Up @@ -243,16 +244,18 @@ export function resolveAliasDeclaration(
// the name of the importer and imported modules so the caller can
// report an error.
let sawPyTypedTransition = false;
let privatePyTypedImported: string | undefined;
let privateImported: string | undefined;
let privatePyTypedImporter: string | undefined;
let privateNonPyTypedImporter: string | undefined;

while (true) {
if (curDeclaration.type !== DeclarationType.Alias || !curDeclaration.symbolName) {
return {
declaration: curDeclaration,
isPrivate,
privatePyTypedImported,
privateImported,
privatePyTypedImporter,
privateNonPyTypedImporter,
};
}

Expand All @@ -262,8 +265,9 @@ export function resolveAliasDeclaration(
return {
declaration: curDeclaration,
isPrivate,
privatePyTypedImported,
privateImported,
privatePyTypedImporter,
privateNonPyTypedImporter,
};
}

Expand Down Expand Up @@ -383,7 +387,24 @@ export function resolveAliasDeclaration(
// symbol that is resolved so we can tell the user to import from this
// location instead.
if (!symbol.isPrivatePyTypedImport()) {
privatePyTypedImported = privatePyTypedImported ?? curDeclaration?.moduleName;
privateImported = privateImported ?? curDeclaration?.moduleName;
}
}
} else {
if (!sawPyTypedTransition) {
if (symbol.isPrivateNonPyTypedImport()) {
privateNonPyTypedImporter = prevDeclaration?.moduleName;
}

// Note that we've seen a transition from a non-py.typed to a py.typed
// import. No further check is needed.
sawPyTypedTransition = true;
} else {
// If we've already seen a transition, look for the first non-private
// symbol that is resolved so we can tell the user to import from this
// location instead.
if (!symbol.isPrivateNonPyTypedImport()) {
privateImported = privateImported ?? curDeclaration?.moduleName;
}
}
}
Expand All @@ -405,8 +426,9 @@ export function resolveAliasDeclaration(
return {
declaration,
isPrivate,
privatePyTypedImported,
privateImported,
privatePyTypedImporter,
privateNonPyTypedImporter,
};
}
alreadyVisited.push(curDeclaration);
Expand Down
11 changes: 11 additions & 0 deletions packages/pyright-internal/src/analyzer/symbol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ export const enum SymbolFlags {

// Indicates that the symbol should be exempt from override type checks.
IgnoredForOverrideChecks = 1 << 12,

// Indicates that the symbol is a private import in a non-py.typed module.
PrivateNonPyTypedImport = 1 << 13,
}

let nextSymbolId = 1;
Expand Down Expand Up @@ -175,6 +178,14 @@ export class Symbol {
return !!(this._flags & SymbolFlags.PrivatePyTypedImport);
}

setPrivateNonPyTypedImport() {
this._flags |= SymbolFlags.PrivateNonPyTypedImport;
}

isPrivateNonPyTypedImport() {
return !!(this._flags & SymbolFlags.PrivateNonPyTypedImport);
}

isNamedTupleMemberMember() {
return !!(this._flags & SymbolFlags.NamedTupleMember);
}
Expand Down
24 changes: 18 additions & 6 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5499,6 +5499,15 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
}),
node.memberName
);
} else if (symbol.isPrivateNonPyTypedImport()) {
addDiagnostic(
DiagnosticRule.reportUnexportedImportUsage,
LocMessage.privateImportFromPyTypedModule().format({
name: memberName,
module: baseType.moduleName,
}),
node.memberName
);
}
} else {
// Does the module export a top-level __getattr__ function?
Expand Down Expand Up @@ -19050,21 +19059,24 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
node.name
);
}

if (resolvedAliasInfo.privatePyTypedImporter) {
const privateImporter =
resolvedAliasInfo.privatePyTypedImporter || resolvedAliasInfo.privateNonPyTypedImporter;
if (privateImporter) {
const diag = new DiagnosticAddendum();
if (resolvedAliasInfo.privatePyTypedImported) {
if (resolvedAliasInfo.privateImported) {
diag.addMessage(
LocAddendum.privateImportFromPyTypedSource().format({
module: resolvedAliasInfo.privatePyTypedImported,
module: resolvedAliasInfo.privateImported,
})
);
}
addDiagnostic(
DiagnosticRule.reportPrivateImportUsage,
resolvedAliasInfo.privatePyTypedImporter
? DiagnosticRule.reportPrivateImportUsage
: DiagnosticRule.reportUnexportedImportUsage,
LocMessage.privateImportFromPyTypedModule().format({
name: node.name.value,
module: resolvedAliasInfo.privatePyTypedImporter,
module: privateImporter,
}) + diag.getString(),
node.name
);
Expand Down
8 changes: 8 additions & 0 deletions packages/pyright-internal/src/common/configOptions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,9 @@ export interface DiagnosticRuleSet {

// Report ignore comments without a specified rule
reportIgnoreCommentWithoutRule: DiagnosticLevel;

// the "works properly" version of reportPrivateImportUsage
reportUnexportedImportUsage: DiagnosticLevel;
}

export function cloneDiagnosticRuleSet(diagSettings: DiagnosticRuleSet): DiagnosticRuleSet {
Expand Down Expand Up @@ -618,6 +621,7 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnreachable: 'none',
reportAny: 'none',
reportIgnoreCommentWithoutRule: 'none',
reportUnexportedImportUsage: 'none',
};

return diagSettings;
Expand Down Expand Up @@ -723,6 +727,7 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnreachable: 'none',
reportAny: 'none',
reportIgnoreCommentWithoutRule: 'none',
reportUnexportedImportUsage: 'none',
};

return diagSettings;
Expand Down Expand Up @@ -828,6 +833,7 @@ export function getStandardDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnreachable: 'none',
reportAny: 'none',
reportIgnoreCommentWithoutRule: 'none',
reportUnexportedImportUsage: 'none',
};

return diagSettings;
Expand Down Expand Up @@ -932,6 +938,7 @@ export const getAllDiagnosticRuleSet = (): DiagnosticRuleSet => ({
reportUnreachable: 'error',
reportAny: 'error',
reportIgnoreCommentWithoutRule: 'error',
reportUnexportedImportUsage: 'error',
});

export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
Expand Down Expand Up @@ -1034,6 +1041,7 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnreachable: 'none',
reportAny: 'none',
reportIgnoreCommentWithoutRule: 'none',
reportUnexportedImportUsage: 'none',
};

return diagSettings;
Expand Down
1 change: 1 addition & 0 deletions packages/pyright-internal/src/common/diagnosticRules.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,5 @@ export enum DiagnosticRule {
reportUnreachable = 'reportUnreachable',
reportAny = 'reportAny',
reportIgnoreCommentWithoutRule = 'reportIgnoreCommentWithoutRule',
reportUnexportedImportUsage = 'reportUnexportedImportUsage',
}
16 changes: 16 additions & 0 deletions packages/vscode-pyright/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,22 @@
true,
false
]
},
"reportUnexportedImportUsage": {
"type": [
"string",
"boolean"
],
"description": "Diagnostics for incorrect usage of symbol imported from a non-\"py.typed\" module that is not re-exported from that module. Should be used along with `reportNonPrivateImportUsage`",
"default": "none",
"enum": [
"none",
"information",
"warning",
"error",
true,
false
]
}
}
},
Expand Down
6 changes: 6 additions & 0 deletions packages/vscode-pyright/schemas/pyrightconfig.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,12 @@
"title": "Controls reporting `# type: ignore` and `# pyright: ignore` comments without specifying a rule",
"default": "none"
},
"reportUnexportedImportUsage": {
"$id": "#/properties/reportUnexportedImportUsage",
"$ref": "#/definitions/diagnostic",
"title": "Controls reporting of improper usage of symbols imported from a non-\"py.typed\" module that are not re-exported from that module. Should be used along with `reportPrivateImportUsage`",
"default": "none"
},
"extraPaths": {
"$id": "#/properties/extraPaths",
"type": "array",
Expand Down

0 comments on commit 5cb08f6

Please sign in to comment.