microsoft/pyright
1
1
Fork 0
mirror of https://github.com/microsoft/pyright.git synced 2024-10-03 19:37:39 +03:00
Code Issues Projects Releases Wiki Activity

Improved diagnostic messages for type errors detected during __set__ and __delete__ calls to a descriptor object. This addresses #8181. (#8198)

Browse Source
This commit is contained in:
Eric Traut 2024-06-21 23:13:01 +02:00 committed by GitHub
parent d2ff2a4517
commit 2efc12f999
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 101 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

162
packages/pyright-internal/src/analyzer/typeEvaluator.ts
View File

@ -578,6 +578,11 @@ interface CodeFlowAnalyzerCacheEntry {
type LogWrapper = <T extends (...args: any[]) => any>(func: T) => (...args: Parameters<T>) => ReturnType<T>;
interface SuppressedNodeStackEntry {
node: ParseNode;
suppressedDiags: string[] | undefined;
}
export function createTypeEvaluator(
importLookup: ImportLookup,
evaluatorOptions: EvaluatorOptions,
@ -586,7 +591,7 @@ export function createTypeEvaluator(
const symbolResolutionStack: SymbolResolutionStackEntry[] = [];
const asymmetricAccessorAssignmentCache = new Set<number>();
const speculativeTypeTracker = new SpeculativeTypeTracker();
const suppressedNodeStack: ParseNode[] = [];
const suppressedNodeStack: SuppressedNodeStackEntry[] = [];
const assignClassToSelfStack: AssignClassToSelfInfo[] = [];
let functionRecursionMap = new Set<number>();
@ -3171,21 +3176,55 @@ export function createTypeEvaluator(
node: ParseNode,
range?: TextRange
) {
if (!isDiagnosticSuppressedForNode(node) && isNodeReachable(node)) {
if (isDiagnosticSuppressedForNode(node)) {
// See if this node is suppressed but the diagnostic should be generated
// anyway so it can be used by the caller that requested the suppression.
const suppressionEntry = suppressedNodeStack.find(
(suppressedNode) =>
ParseTreeUtils.isNodeContainedWithin(node, suppressedNode.node) && suppressedNode.suppressedDiags
);
suppressionEntry?.suppressedDiags?.push(message);
return undefined;
}
if (isNodeReachable(node)) {
const fileInfo = AnalyzerNodeInfo.getFileInfo(node);
return fileInfo.diagnosticSink.addDiagnosticWithTextRange(diagLevel, message, range || node);
return fileInfo.diagnosticSink.addDiagnosticWithTextRange(diagLevel, message, range ?? node);
}
return undefined;
}
function isDiagnosticSuppressedForNode(node: ParseNode) {
return (
suppressedNodeStack.some((suppressedNode) => ParseTreeUtils.isNodeContainedWithin(node, suppressedNode)) ||
speculativeTypeTracker.isSpeculative(node, /* ignoreIfDiagnosticsAllowed */ true)
if (speculativeTypeTracker.isSpeculative(node, /* ignoreIfDiagnosticsAllowed */ true)) {
return true;
}
return suppressedNodeStack.some((suppressedNode) =>
ParseTreeUtils.isNodeContainedWithin(node, suppressedNode.node)
);
}
// This function is similar to isDiagnosticSuppressedForNode except that it
// returns false if diagnostics are suppressed for the node but the caller
// has requested that diagnostics be generated anyway.
function canSkipDiagnosticForNode(node: ParseNode) {
if (speculativeTypeTracker.isSpeculative(node, /* ignoreIfDiagnosticsAllowed */ true)) {
return true;
}
const suppressedEntries = suppressedNodeStack.filter((suppressedNode) =>
ParseTreeUtils.isNodeContainedWithin(node, suppressedNode.node)
);
if (suppressedEntries.length === 0) {
return false;
}
return suppressedEntries.every((entry) => !entry.suppressedDiags);
}
function addDiagnostic(rule: DiagnosticRule, message: string, node: ParseNode, range?: TextRange) {
const fileInfo = AnalyzerNodeInfo.getFileInfo(node);
const diagLevel = fileInfo.diagnosticRuleSet[rule] as DiagnosticLevel;
@ -6123,17 +6162,29 @@ export function createTypeEvaluator(
}
// Suppress diagnostics for these method calls because they would be redundant.
const callResult = suppressDiagnostics(errorNode, () => {
return validateCallArguments(
errorNode,
argList,
{ type: methodType },
/* typeVarContext */ undefined,
/* skipUnknownArgCheck */ true,
/* inferenceContext */ undefined,
/* signatureTracker */ undefined
);
});
const callResult = suppressDiagnostics(
errorNode,
() => {
return validateCallArguments(
errorNode,
argList,
{ type: methodType },
/* typeVarContext */ undefined,
/* skipUnknownArgCheck */ true,
/* inferenceContext */ undefined,
/* signatureTracker */ undefined
);
},
(suppressedDiags) => {
// If diagnostics were recorded when suppressed, add them to the
// diagnostic as messages.
if (diag) {
suppressedDiags.forEach((message) => {
diag?.addMessageMultiline(message);
});
}
}
);
// Collect deprecation information associated with the member access method.
let deprecationInfo: MemberAccessDeprecationInfo | undefined;
@ -6158,34 +6209,6 @@ export function createTypeEvaluator(
};
}
// Errors were detected when evaluating the access method call.
if (usage.method === 'set') {
if (
usage.setType &&
isFunction(methodType) &&
methodType.details.parameters.length >= 2 &&
!usage.setType.isIncomplete
) {
const setterType = FunctionType.getEffectiveParameterType(methodType, 1);
diag?.addMessage(
LocAddendum.typeIncompatible().format({
destType: printType(setterType),
sourceType: printType(usage.setType.type),
})
);
} else if (isOverloadedFunction(methodType)) {
diag?.addMessage(LocMessage.noOverload().format({ name: accessMethodName }));
}
} else {
diag?.addMessage(
LocAddendum.descriptorAccessCallFailed().format({
name: accessMethodName,
className: printType(convertToInstance(methodClassType)),
})
);
}
return {
type: UnknownType.create(),
typeErrors: true,
@ -9001,7 +9024,7 @@ export function createTypeEvaluator(
if (filteredMatchResults.length === 0) {
// Skip the error message if we're in speculative mode because it's very
// expensive, and we're going to suppress the diagnostic anyway.
if (!isDiagnosticSuppressedForNode(errorNode)) {
if (!canSkipDiagnosticForNode(errorNode)) {
const functionName = typeResult.type.overloads[0].details.name || '<anonymous function>';
const diagAddendum = new DiagnosticAddendum();
const argTypes = argList.map((t) => {
@ -9144,7 +9167,7 @@ export function createTypeEvaluator(
// We couldn't find any valid overloads. Skip the error message if we're
// in speculative mode because it's very expensive, and we're going to
// suppress the diagnostic anyway.
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
const result = evaluateUsingBestMatchingOverload(
/* skipUnknownArgCheck */ true,
/* emitNoOverloadFoundError */ true
@ -10328,7 +10351,7 @@ export function createTypeEvaluator(
}
if (tooManyPositionals) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
positionParamLimitIndex === 1
@ -10379,7 +10402,7 @@ export function createTypeEvaluator(
argTypeResult.type.paramSpecAccess === 'args' &&
paramDetails.params[paramIndex].param.category !== ParameterCategory.ArgsList
) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
positionParamLimitIndex === 1
@ -10458,7 +10481,7 @@ export function createTypeEvaluator(
// It's not allowed to use unpacked arguments with a variadic *args
// parameter unless the argument is a variadic arg as well.
if (isParamVariadic && !isArgCompatibleWithVariadic) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
LocMessage.unpackedArgWithVariadicParam(),
@ -10531,7 +10554,7 @@ export function createTypeEvaluator(
if (remainingArgCount <= remainingParamCount) {
if (remainingArgCount < remainingParamCount) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
// Have we run out of arguments and still have parameters left to fill?
addDiagnostic(
DiagnosticRule.reportCallIssue,
@ -10633,7 +10656,7 @@ export function createTypeEvaluator(
}
if (argsRemainingCount > 0) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
argsRemainingCount === 1
@ -10729,7 +10752,7 @@ export function createTypeEvaluator(
});
if (!diag.isEmpty()) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
LocMessage.unpackedTypedDictArgument() + diag.getString(),
@ -10807,7 +10830,7 @@ export function createTypeEvaluator(
}
if (!isValidMappingType) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
LocMessage.unpackedDictArgumentNotMapping(),
@ -10832,7 +10855,7 @@ export function createTypeEvaluator(
const paramEntry = paramMap.get(paramNameValue);
if (paramEntry && !paramEntry.isPositionalOnly) {
if (paramEntry.argsReceived > 0) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
LocMessage.paramAlreadyAssigned().format({ name: paramNameValue }),
@ -10884,7 +10907,7 @@ export function createTypeEvaluator(
);
trySetActive(argList[argIndex], paramDetails.params[paramDetails.kwargsIndex].param);
} else {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
LocMessage.paramNameMissing().format({ name: paramName.value }),
@ -10897,7 +10920,7 @@ export function createTypeEvaluator(
if (paramSpecArgList) {
paramSpecArgList.push(argList[argIndex]);
} else {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
positionParamLimitIndex === 1
@ -10989,9 +11012,9 @@ export function createTypeEvaluator(
});
if (unassignedParams.length > 0) {
if (!isDiagnosticSuppressedForNode(errorNode)) {
if (!canSkipDiagnosticForNode(errorNode)) {
const missingParamNames = unassignedParams.map((p) => `"${p}"`).join(', ');
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
unassignedParams.length === 1
@ -11083,7 +11106,7 @@ export function createTypeEvaluator(
argParam.argument.argumentCategory !== ArgumentCategory.UnpackedList &&
!argParam.mapsToVarArgList
) {
if (!isDiagnosticSuppressedForNode(errorNode) && !isTypeIncomplete) {
if (!canSkipDiagnosticForNode(errorNode) && !isTypeIncomplete) {
addDiagnostic(
DiagnosticRule.reportCallIssue,
LocMessage.typeVarTupleMustBeUnpacked(),
@ -12212,7 +12235,7 @@ export function createTypeEvaluator(
const fileInfo = AnalyzerNodeInfo.getFileInfo(argParam.errorNode);
if (
fileInfo.diagnosticRuleSet.reportArgumentType !== 'none' &&
!isDiagnosticSuppressedForNode(argParam.errorNode) &&
!canSkipDiagnosticForNode(argParam.errorNode) &&
!isTypeIncomplete
) {
const argTypeText = printType(argType);
@ -14440,9 +14463,7 @@ export function createTypeEvaluator(
{
dependentType: inferenceContext?.expectedType,
allowDiagnostics:
!forceSpeculative &&
!isDiagnosticSuppressedForNode(node) &&
!inferenceContext?.isTypeIncomplete,
!forceSpeculative && !canSkipDiagnosticForNode(node) && !inferenceContext?.isTypeIncomplete,
}
);
@ -20659,12 +20680,19 @@ export function createTypeEvaluator(
}
// Disables recording of errors and warnings.
function suppressDiagnostics<T>(node: ParseNode, callback: () => T) {
suppressedNodeStack.push(node);
function suppressDiagnostics<T>(
node: ParseNode,
callback: () => T,
diagCallback?: (suppressedDiags: string[]) => void
) {
suppressedNodeStack.push({ node, suppressedDiags: diagCallback ? [] : undefined });
try {
const result = callback();
suppressedNodeStack.pop();
const poppedNode = suppressedNodeStack.pop();
if (diagCallback && poppedNode?.suppressedDiags) {
diagCallback(poppedNode.suppressedDiags);
}
return result;
} catch (e) {
// We don't use finally here because the TypeScript debugger doesn't

6
packages/pyright-internal/src/common/diagnostic.ts
View File

@ -165,6 +165,12 @@ export class DiagnosticAddendum {
this._messages.push(message);
}
addMessageMultiline(message: string) {
message.split('\n').forEach((line) => {
this._messages.push(line);
});
}
addTextRange(range: TextRange) {
this._range = range;
}