Skip to content

Commit

Permalink
Eliminated some unnecessary work when determining the effective type …
Browse files Browse the repository at this point in the history 
…of a symbol that doesn't have a declared type. When evaluated in the context of a particular usage, it's unnecessary to evaluate any assignments within the same execution scope. (microsoft#6786)
  • Loading branch information
@erictraut
erictraut committed Dec 20, 2023
1 parent 7d0f48a commit 27ff8c8
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 45 deletions.
6 changes: 5 additions & 1 deletion packages/pyright-internal/src/analyzer/codeFlowEngine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,11 @@ export function getCodeFlowEngine(

if (flowTypeResult) {
if (isTypeAliasPlaceholder(flowTypeResult.type)) {
flowTypeResult = undefined;
// Don't cache a recursive type alias placeholder.
return {
type: flowTypeResult.type,
isIncomplete: true,
};
} else if (
reference.nodeType === ParseNodeType.MemberAccess &&
evaluator.isAsymmetricAccessorAssignment(targetNode)
Expand Down
86 changes: 46 additions & 40 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20931,14 +20931,15 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions

// Returns the type of the symbol. If the type is explicitly declared, that type
// is returned. If not, the type is inferred from assignments to the symbol. All
// assigned types are evaluated and combined into a union. If a "usageNode"
// node is specified, only declarations that are outside of the current execution
// scope or that are reachable (as determined by code flow analysis) are considered.
// This helps in cases where there are cyclical dependencies between symbols.
// assigned types are evaluated and combined into a union.
function getEffectiveTypeOfSymbol(symbol: Symbol): Type {
return getEffectiveTypeOfSymbolForUsage(symbol).type;
}

// If a "usageNode" node is specified, only declarations that are outside
// of the current execution scope or that are reachable (as determined by
// code flow analysis) are considered. This helps in cases where there
// are cyclical dependencies between symbols.
function getEffectiveTypeOfSymbolForUsage(
symbol: Symbol,
usageNode?: NameNode,
Expand All @@ -20949,22 +20950,22 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
// If there's a declared type, it takes precedence over inferred types.
if (symbol.hasTypedDeclarations()) {
declaredTypeInfo = getDeclaredTypeOfSymbol(symbol, usageNode);

const declaredType = declaredTypeInfo?.type;
const hasMetadata = !!declaredTypeInfo.isTypeAlias;

if (declaredType || !hasMetadata) {
let isIncomplete = false;

if (declaredType) {
if (isFunction(declaredType) && FunctionType.isPartiallyEvaluated(declaredType)) {
isIncomplete = true;
} else if (isClass(declaredType) && ClassType.isPartiallyEvaluated(declaredType)) {
isIncomplete = true;
}
let isIncomplete = false;
if (declaredType) {
if (isFunction(declaredType) && FunctionType.isPartiallyEvaluated(declaredType)) {
isIncomplete = true;
} else if (isClass(declaredType) && ClassType.isPartiallyEvaluated(declaredType)) {
isIncomplete = true;
}
}

// If the "declared" type uses a "TypeAlias" type annotation, then
// we need to use the inferred type path to evaluate its type.
if (declaredType || !declaredTypeInfo.isTypeAlias) {
const typedDecls = symbol.getTypedDeclarations();

const result: EffectiveTypeResult = {
type: declaredType ?? UnknownType.create(),
isIncomplete,
Expand All @@ -20978,6 +20979,10 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
}
}

return inferTypeOfSymbolForUsage(symbol, usageNode, useLastDecl);
}

function inferTypeOfSymbolForUsage(symbol: Symbol, usageNode?: NameNode, useLastDecl = false): EffectiveTypeResult {
// Look in the inferred type cache to see if we've computed this already.
let cacheEntries = effectiveTypeCache.get(symbol.id);
const usageNodeId = usageNode ? usageNode.id : undefined;
Expand Down Expand Up @@ -21035,9 +21040,29 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions

// Determine which declarations to use for inference.
const declsToConsider: Declaration[] = [];
let includesVariableDecl = false;
let includesIllegalTypeAliasDecl = false;

let sawExplicitTypeAlias = false;
decls.forEach((decl, index) => {
const resolvedDecl =
resolveAliasDeclaration(decl, /* resolveLocalNames */ true, {
allowExternallyHiddenAccess: AnalyzerNodeInfo.getFileInfo(decl.node).isStubFile,
}) ?? decl;

if (!isPossibleTypeAliasDeclaration(resolvedDecl)) {
includesIllegalTypeAliasDecl = true;
}

if (resolvedDecl.type === DeclarationType.Variable) {
// Exempt typing.pyi, which uses variables to define some
// special forms like Any.
const fileInfo = AnalyzerNodeInfo.getFileInfo(resolvedDecl.node);
if (!fileInfo.isTypingStubFile) {
includesVariableDecl = true;
}
}

if (declIndexToConsider !== undefined && declIndexToConsider !== index) {
return;
}
Expand All @@ -21060,21 +21085,16 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
if (usageNode !== undefined) {
if (decl.type !== DeclarationType.Alias) {
// Is the declaration in the same execution scope as the "usageNode" node?
// If so, we can skip it because code flow analysis will allow us
// to determine the type in this context.
const usageScope = ParseTreeUtils.getExecutionScopeNode(usageNode);
const declScope = ParseTreeUtils.getExecutionScopeNode(decl.node);
if (usageScope === declScope) {
if (!isFlowPathBetweenNodes(decl.node, usageNode)) {
return;
}
return;
}
}
}

const resolvedDecl =
resolveAliasDeclaration(decl, /* resolveLocalNames */ true, {
allowExternallyHiddenAccess: AnalyzerNodeInfo.getFileInfo(decl.node).isStubFile,
}) ?? decl;

const isExplicitTypeAlias = isExplicitTypeAliasDeclaration(resolvedDecl);
const isTypeAlias = isExplicitTypeAlias || isPossibleTypeAliasOrTypedDict(resolvedDecl);

Expand All @@ -21096,6 +21116,8 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
});

const result = getTypeOfSymbolForDecls(symbol, declsToConsider, effectiveTypeCacheKey);
result.includesVariableDecl = includesVariableDecl;
result.includesIllegalTypeAliasDecl = includesIllegalTypeAliasDecl;

// Add the result to the effective type cache if it doesn't include speculative results.
if (!result.includesSpeculativeResult) {
Expand All @@ -21120,7 +21142,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
const typesToCombine: Type[] = [];
let isIncomplete = false;
let sawPendingEvaluation = false;
let includesVariableDecl = false;
let includesSpeculativeResult = false;

decls.forEach((decl) => {
Expand All @@ -21134,13 +21155,6 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions

if (type) {
if (decl.type === DeclarationType.Variable) {
// Exempt typing.pyi, which uses variables to define some
// special forms like Any.
const fileInfo = AnalyzerNodeInfo.getFileInfo(decl.node);
if (!fileInfo.isTypingStubFile) {
includesVariableDecl = true;
}

let isConstant = false;
if (decl.type === DeclarationType.Variable) {
if (decl.isConstant || isFinalVariableDeclaration(decl)) {
Expand Down Expand Up @@ -21211,15 +21225,7 @@ export function createTypeEvaluator(importLookup: ImportLookup, evaluatorOptions
type = UnboundType.create();
}

return {
type,
isIncomplete,
includesVariableDecl,
includesIllegalTypeAliasDecl: !decls.every((decl) => isPossibleTypeAliasDeclaration(decl)),
includesSpeculativeResult,
isRecursiveDefinition: false,
evaluationAttempts,
};
return { type, isIncomplete, includesSpeculativeResult, evaluationAttempts };
}

// If a declaration has an explicit type (e.g. a variable with an annotation),
Expand Down
8 changes: 4 additions & 4 deletions packages/pyright-internal/src/analyzer/typeEvaluatorTypes.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,10 @@ export type FunctionArgument = FunctionArgumentWithType | FunctionArgumentWithEx
export interface EffectiveTypeResult {
type: Type;
isIncomplete: boolean;
includesVariableDecl: boolean;
includesIllegalTypeAliasDecl: boolean;
includesSpeculativeResult: boolean;
isRecursiveDefinition: boolean;
includesVariableDecl?: boolean;
includesIllegalTypeAliasDecl?: boolean;
includesSpeculativeResult?: boolean;
isRecursiveDefinition?: boolean;
evaluationAttempts?: number;
}

Expand Down

0 comments on commit 27ff8c8

Please sign in to comment.