Skip to content

Commit

Permalink
Fixed a bug that leads to an infinite loop when performing protocol m…
Browse files Browse the repository at this point in the history
…atching under certain circumstances that involve recursive protocol definitions. This addresses #7786.
  • Loading branch information
erictraut committed Apr 28, 2024
1 parent 1622abb commit 2870e25
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
14 changes: 9 additions & 5 deletions packages/pyright-internal/src/analyzer/protocols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/

import { assert } from '../common/debug';
import { DiagnosticAddendum } from '../common/diagnostic';
import { defaultMaxDiagnosticDepth, DiagnosticAddendum } from '../common/diagnostic';
import { LocAddendum } from '../localization/localize';
import { assignTypeToTypeVar } from './constraintSolver';
import { DeclarationType } from './declaration';
Expand Down Expand Up @@ -65,7 +65,7 @@ interface ProtocolCompatibility {
const protocolAssignmentStack: ProtocolAssignmentStackEntry[] = [];

// Maximum number of different types that are cached with a protocol.
const maxProtocolCompatibilityCacheEntries = 32;
const maxProtocolCompatibilityCacheEntries = 64;

export function assignClassToProtocol(
evaluator: TypeEvaluator,
Expand Down Expand Up @@ -108,9 +108,13 @@ export function assignClassToProtocol(
}

// If it's known not to be compatible and the caller hasn't requested
// any detailed diagnostic information, we can return false immediately.
if (!compatibility && !diag) {
return false;
// any detailed diagnostic information or we've already exceeded the
// depth of diagnostic information that will be displayed, we can
// return false immediately.
if (!compatibility) {
if (!diag || diag.getNestLevel() > defaultMaxDiagnosticDepth) {
return false;
}
}
}
}
Expand Down
16 changes: 13 additions & 3 deletions packages/pyright-internal/src/common/diagnostic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import { DiagnosticLevel } from './configOptions';
import { Range, TextRange } from './textRange';
import { Uri } from './uri/uri';

const defaultMaxDepth = 5;
const defaultMaxLineCount = 8;
export const defaultMaxDiagnosticDepth = 5;
export const defaultMaxDiagnosticLineCount = 8;
const maxRecursionCount = 64;

// Corresponds to the CommentTaskPriority enum at https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_git/VS?path=src/env/shell/PackageFramework/Framework/CommentTaskPriority.cs
Expand Down Expand Up @@ -151,6 +151,11 @@ export class DiagnosticAddendum {
private _messages: string[] = [];
private _childAddenda: DiagnosticAddendum[] = [];

// The nest level is accurate only for the common case where all
// addendum are created using createAddendum. This is an upper bound.
// The actual nest level may be smaller.
private _nestLevel: number | undefined;

// Addenda normally don't have their own ranges, but there are cases
// where we want to track ranges that can influence the range of the
// diagnostic.
Expand All @@ -167,11 +172,12 @@ export class DiagnosticAddendum {
// Create a new (nested) addendum to which messages can be added.
createAddendum() {
const newAddendum = new DiagnosticAddendum();
newAddendum._nestLevel = (this._nestLevel ?? 0) + 1;
this.addAddendum(newAddendum);
return newAddendum;
}

getString(maxDepth = defaultMaxDepth, maxLineCount = defaultMaxLineCount): string {
getString(maxDepth = defaultMaxDiagnosticDepth, maxLineCount = defaultMaxDiagnosticLineCount): string {
let lines = this._getLinesRecursive(maxDepth, maxLineCount);

if (lines.length > maxLineCount) {
Expand Down Expand Up @@ -203,6 +209,10 @@ export class DiagnosticAddendum {
return this._messages;
}

getNestLevel() {
return this._nestLevel ?? 0;
}

// Returns undefined if no range is associated with this addendum
// or its children. Returns a non-empty range if there is a single range
// associated.
Expand Down

0 comments on commit 2870e25

Please sign in to comment.