Skip to content

feat(compiler): Add fixedOrder option to expectDiagnostics #7453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/flexible-diagnostics.md
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use something else than changeset for this repo called chronus. Either follow the comment that should get added soon or use pnpm chronus add

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@typespec/compiler": minor
---

Add fixedOrder option to expectDiagnostics to allow matching diagnostics in any order
218 changes: 178 additions & 40 deletions packages/compiler/src/testing/expect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,86 +56,224 @@ export interface DiagnosticMatch {
/**
* Validate the diagnostic array contains exactly the given diagnostics.
* @param diagnostics Array of the diagnostics
* @param match The diagnostic match(es) to validate against
* @param options Options for validation
* @param options.strict When true, expects exactly the same number of diagnostics as matches
* @param options.fixedOrder When true, diagnostics must appear in the same order as matches. When false, order is ignored.
*/
export function expectDiagnostics(
diagnostics: readonly Diagnostic[],
match: DiagnosticMatch | DiagnosticMatch[],
options = {
strict: true,
},
options: {
strict?: boolean;
fixedOrder?: boolean;
} = {},
) {
// Set defaults
const { strict = true, fixedOrder = true } = options;

const array = isArray(match) ? match : [match];

if (options.strict && array.length !== diagnostics.length) {
if (strict && array.length !== diagnostics.length) {
fail(
`Expected ${array.length} diagnostics but found ${diagnostics.length}:\n ${formatDiagnostics(
diagnostics,
)}`,
);
}
for (let i = 0; i < array.length; i++) {
const diagnostic = diagnostics[i];
const expectation = array[i];
const sep = "-".repeat(100);
const message = `Diagnostics found:\n${sep}\n${formatDiagnostics(diagnostics)}\n${sep}`;
if (expectation.code !== undefined) {
strictEqual(
diagnostic.code,
expectation.code,
`Diagnostic at index ${i} has non matching code.\n${message}`,

if (fixedOrder) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach we decided here was to sort the diagnostics so they diff can still be used.
The reason I didn't want to add this at it was was exactly because it made the test experience worse when it didn't find the match and in general it was bad pattern to have multiple diagnostics tested at the same time. We decided that sorting the diagnositcs and expectation would still provide the benefits while not loosing the diff

// Original behavior: match diagnostics in order
for (let i = 0; i < array.length; i++) {
const diagnostic = diagnostics[i];
const expectation = array[i];
validateDiagnosticMatch(diagnostic, expectation, i, diagnostics);
}
} else {
// New behavior: match diagnostics in any order
const remainingDiagnostics = [...diagnostics];
const unmatchedExpectations: number[] = [];

for (let i = 0; i < array.length; i++) {
const expectation = array[i];
let matchFound = false;

for (let j = remainingDiagnostics.length - 1; j >= 0; j--) {
const diagnostic = remainingDiagnostics[j];
if (doesDiagnosticMatch(diagnostic, expectation)) {
remainingDiagnostics.splice(j, 1);
matchFound = true;
break;
}
}

if (!matchFound) {
unmatchedExpectations.push(i);
}
}

if (unmatchedExpectations.length > 0) {
const sep = "-".repeat(100);
const message = `Diagnostics found:\n${sep}\n${formatDiagnostics(diagnostics)}\n${sep}`;
fail(
`Could not find matches for expectations at indices: ${unmatchedExpectations.join(", ")}.\n${message}`,
);
}

if (expectation.message !== undefined) {
// When strict mode is enabled but fixedOrder is false, check for extra diagnostics
if (strict && remainingDiagnostics.length > 0) {
const sep = "-".repeat(100);
const message = `Extra unmatched diagnostics found:\n${sep}\n${formatDiagnostics(remainingDiagnostics)}\n${sep}`;
fail(message);
}
}
}

/**
* Validates that a diagnostic matches the expected criteria
*/
function validateDiagnosticMatch(
diagnostic: Diagnostic,
expectation: DiagnosticMatch,
index: number,
allDiagnostics: readonly Diagnostic[],
) {
const sep = "-".repeat(100);
const message = `Diagnostics found:\n${sep}\n${formatDiagnostics(allDiagnostics)}\n${sep}`;

if (expectation.code !== undefined) {
strictEqual(
diagnostic.code,
expectation.code,
`Diagnostic at index ${index} has non matching code.\n${message}`,
);
}

if (expectation.message !== undefined) {
matchStrOrRegex(
diagnostic.message,
expectation.message,
`Diagnostic at index ${index} has non matching message.\n${message}`,
);
}
if (expectation.severity !== undefined) {
strictEqual(
diagnostic.severity,
expectation.severity,
`Diagnostic at index ${index} has non matching severity.\n${message}`,
);
}
if (
expectation.file !== undefined ||
expectation.pos !== undefined ||
expectation.end !== undefined
) {
if (diagnostic.target === NoTarget) {
fail(`Diagnostics at index ${index} expected to have a target.\n${message}`);
}
const source = getSourceLocation(diagnostic.target);

if (!source) {
fail(`Diagnostics at index ${index} expected to have a valid source location.\n${message}`);
return; // This will never be reached, but helps TypeScript understand control flow
}

if (expectation.file !== undefined) {
matchStrOrRegex(
diagnostic.message,
expectation.message,
`Diagnostic at index ${i} has non matching message.\n${message}`,
source.file.path,
typeof expectation.file === "string"
? resolveVirtualPath(expectation.file)
: expectation.file,
`Diagnostics at index ${index} has non matching file.\n${message}`,
);
}

if (expectation.pos !== undefined) {
strictEqual(
source.pos,
expectation.pos,
`Diagnostic at index ${index} has non-matching start position.`,
);
}
if (expectation.severity !== undefined) {

if (expectation.end !== undefined) {
strictEqual(
diagnostic.severity,
expectation.severity,
`Diagnostic at index ${i} has non matching severity.\n${message}`,
source.end,
expectation.end,
`Diagnostic at index ${index} has non-matching end position.`,
);
}
}
}

/**
* Checks if a diagnostic matches the expected criteria without throwing errors
*/
function doesDiagnosticMatch(diagnostic: Diagnostic, expectation: DiagnosticMatch): boolean {
try {
if (expectation.code !== undefined && diagnostic.code !== expectation.code) {
return false;
}

if (expectation.message !== undefined) {
if (typeof expectation.message === "string") {
if (diagnostic.message !== expectation.message) {
return false;
}
} else {
if (!expectation.message.test(diagnostic.message)) {
return false;
}
}
}

if (expectation.severity !== undefined && diagnostic.severity !== expectation.severity) {
return false;
}

if (
expectation.file !== undefined ||
expectation.pos !== undefined ||
expectation.end !== undefined
) {
if (diagnostic.target === NoTarget) {
fail(`Diagnostics at index ${i} expected to have a target.\n${message}`);
return false;
}
const source = getSourceLocation(diagnostic.target);

if (!source) {
return false;
}

if (expectation.file !== undefined) {
matchStrOrRegex(
source.file.path,
const expectedFile =
typeof expectation.file === "string"
? resolveVirtualPath(expectation.file)
: expectation.file,
`Diagnostics at index ${i} has non matching file.\n${message}`,
);
: expectation.file;

if (typeof expectedFile === "string") {
if (source.file.path !== expectedFile) {
return false;
}
} else {
if (!expectedFile.test(source.file.path)) {
return false;
}
}
}

if (expectation.pos !== undefined) {
strictEqual(
source.pos,
expectation.pos,
`Diagnostic at index ${i} has non-matching start position.`,
);
if (expectation.pos !== undefined && source.pos !== expectation.pos) {
return false;
}

if (expectation.end !== undefined) {
strictEqual(
source.end,
expectation.end,
`Diagnostic at index ${i} has non-matching end position.`,
);
if (expectation.end !== undefined && source.end !== expectation.end) {
return false;
}
}

return true;
} catch {
return false;
}
}

Expand Down
Loading