Skip to content

Commit fa3a5c5

Browse files
timotheeguerintimotheeguerin
authored
Remove autorest emit that shouldn't exists and update tsv linting to allow generic suppression (#43220)
1 parent 7fb7de0 commit fa3a5c5

9 files changed

Lines changed: 327 additions & 220 deletions

File tree

‎eng/tools/typespec-validation/src/index.ts‎

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { stat } from "node:fs/promises";
22
import { ParseArgsConfig, parseArgs } from "node:util";
33
import { Suppression } from "suppressions";
4+
import { Rule } from "./rule.js";
45
import { CompileRule } from "./rules/compile.js";
56
import { EmitAutorestRule } from "./rules/emit-autorest.js";
67
import { FlavorAzureRule } from "./rules/flavor-azure.js";
@@ -14,6 +15,56 @@ import { fileExists, getSuppressions, normalizePath } from "./utils.js";
1415
// Context argument may add new properties or override checkingAllSpecs
1516
export let context: Record<string, unknown> = { checkingAllSpecs: false };
1617

18+
export interface RunRulesResult {
19+
success: boolean;
20+
suppressed: string[];
21+
executed: string[];
22+
failed: string[];
23+
}
24+
25+
/**
26+
* Runs the given rules against a folder, handling per-rule suppressions
27+
* for rules that opt in via `suppressable: true`.
28+
*/
29+
export async function runRules(
30+
rules: Rule[],
31+
folder: string,
32+
suppressions: Suppression[],
33+
): Promise<RunRulesResult> {
34+
const result: RunRulesResult = { success: true, suppressed: [], executed: [], failed: [] };
35+
36+
for (const rule of rules) {
37+
console.log("\nExecuting rule: " + rule.name);
38+
39+
if (rule.suppressable) {
40+
const ruleSuppressions = suppressions.filter(
41+
(s) => s.rules?.includes(rule.name) && (!s.subRules || s.subRules.length === 0),
42+
);
43+
if (ruleSuppressions.length > 0) {
44+
console.log(` Suppressed: ${ruleSuppressions[0].reason}`);
45+
result.suppressed.push(rule.name);
46+
continue;
47+
}
48+
}
49+
50+
const ruleResult = await rule.execute(folder);
51+
result.executed.push(rule.name);
52+
if (ruleResult.stdOutput) console.log(ruleResult.stdOutput);
53+
if (!ruleResult.success) {
54+
result.success = false;
55+
result.failed.push(rule.name);
56+
console.log("Rule " + rule.name + " failed");
57+
if (ruleResult.errorOutput) console.log(ruleResult.errorOutput);
58+
59+
// Stop executing more rules, since the results are more likely to be confusing than helpful
60+
// Can add property like "RuleResult.ContinueOnError" if some rules want to continue
61+
break;
62+
}
63+
}
64+
65+
return result;
66+
}
67+
1768
export async function main() {
1869
const args = process.argv.slice(2);
1970
const options = {
@@ -56,7 +107,7 @@ export async function main() {
56107
return;
57108
}
58109

59-
const rules = [
110+
const rules: Rule[] = [
60111
new FolderStructureRule(),
61112
new NpmPrefixRule(),
62113
new EmitAutorestRule(),
@@ -66,24 +117,10 @@ export async function main() {
66117
new FormatRule(),
67118
new SdkTspConfigValidationRule(),
68119
];
69-
let success = true;
70-
for (let i = 0; i < rules.length; i++) {
71-
const rule = rules[i];
72-
console.log("\nExecuting rule: " + rule.name);
73-
const result = await rule.execute(absolutePath);
74-
if (result.stdOutput) console.log(result.stdOutput);
75-
if (!result.success) {
76-
success = false;
77-
console.log("Rule " + rule.name + " failed");
78-
if (result.errorOutput) console.log(result.errorOutput);
79120

80-
// Stop executing more rules, since the results are more likely to be confusing than helpful
81-
// Can add property like "RuleResult.ContinueOnError" if some rules want to continue
82-
break;
83-
}
84-
}
121+
const result = await runRules(rules, absolutePath, suppressions);
85122

86-
if (!success) {
123+
if (!result.success) {
87124
process.exitCode = 1;
88125
}
89126
}

‎eng/tools/typespec-validation/src/rule.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,7 @@ export interface Rule {
77
readonly action?: string;
88
// TODO: required when all rules apply it
99
readonly link?: string;
10+
/** When true, the rule runner automatically skips this rule if a matching suppression exists. */
11+
readonly suppressable?: boolean;
1012
execute(folder: string): Promise<RuleResult>;
1113
}

‎eng/tools/typespec-validation/src/rules/compile.ts‎

Lines changed: 130 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,6 @@ export class CompileRule implements Rule {
3434
// Rule output is easier to read if "tsp compile" stderr is redirected to stdOutput
3535
stdOutput += stderr;
3636

37-
if (
38-
stdout.toLowerCase().includes("no emitter was configured") ||
39-
stdout.toLowerCase().includes("no output was generated")
40-
) {
41-
success = false;
42-
errorOutput += "No emitter was configured and/or no output was generated.";
43-
}
44-
4537
if (success) {
4638
if (!err) {
4739
// Check for *extra* typespec-generated swagger files under the output folder, which
@@ -75,145 +67,145 @@ export class CompileRule implements Rule {
7567
stdOutput += outputSwaggers.join("\n") + "\n";
7668

7769
if (outputSwaggers.length === 0) {
78-
throw new Error("No generated swaggers found in output of 'tsp compile'");
79-
}
70+
stdOutput += "No generated swaggers found, skipping extra swagger check.\n";
71+
} else {
72+
// ../resource-manager/Microsoft.Contoso
73+
const outputFolder = dirname(dirname(dirname(outputSwaggers[0])));
74+
const outputFilename = basename(outputSwaggers[0]);
75+
76+
stdOutput += "\nOutput folder:\n";
77+
stdOutput += outputFolder + "\n";
78+
79+
// Filter to only specs matching the folder and filename extracted from the first output-file.
80+
// Necessary to handle multi-project specs like keyvault.
81+
//
82+
// Globby only accepts patterns like posix paths.
83+
const pattern = path.posix.join(...outputFolder.split(path.sep), "**", outputFilename);
84+
const allSwaggers = (await globby(pattern, { ignore: ["**/examples/**"] })).map(
85+
// Globby always returns posix paths
86+
(p) => normalize(p),
87+
);
88+
89+
// Filter to files generated by TypeSpec
90+
const tspGeneratedSwaggers = await filterAsync(
91+
allSwaggers,
92+
async (swaggerPath: string) => {
93+
const swaggerText = await readFile(swaggerPath, { encoding: "utf8" });
94+
const swaggerObj = JSON.parse(swaggerText) as {
95+
info?: Record<string, unknown>;
96+
};
97+
return (
98+
swaggerObj["info"]?.["x-typespec-generated"] !== undefined ||
99+
swaggerObj["info"]?.["x-cadl-generated"] !== undefined
100+
);
101+
},
102+
);
103+
104+
stdOutput += `\nSwaggers matching output folder and filename:\n`;
105+
stdOutput += tspGeneratedSwaggers.join("\n") + "\n";
106+
107+
const suppressedSwaggers = await filterAsync(
108+
tspGeneratedSwaggers,
109+
async (swaggerPath: string) => {
110+
const suppressions = await getSuppressions(swaggerPath);
111+
112+
const extraSwaggerSuppressions = suppressions.filter(
113+
(s) => s.rules?.includes(this.name) && s.subRules?.includes("ExtraSwagger"),
114+
);
115+
116+
// Each path must specify a single version (without wildcards) under "preview|stable"
117+
//
118+
// Allowed: data-plane/Azure.Contoso.WidgetManager/preview/2022-11-01-preview/**/*.json
119+
// Disallowed: data-plane/Azure.Contoso.WidgetManager/preview/**/*.json
120+
// Disallowed: data-plane/**/*.json
121+
//
122+
// Include "." since a few specs use versions like "X.Y" instead of "YYYY-MM-DD"
123+
const singleVersionPattern = "/(preview|stable)/[A-Za-z0-9._-]+/";
124+
125+
for (const suppression of extraSwaggerSuppressions) {
126+
for (const p of suppression.paths) {
127+
if (!p.match(singleVersionPattern)) {
128+
throw new Error(
129+
`Invalid path '${p}'. Path must only include one version per suppression.`,
130+
);
131+
}
132+
}
133+
}
80134

81-
// ../resource-manager/Microsoft.Contoso
82-
const outputFolder = dirname(dirname(dirname(outputSwaggers[0])));
83-
const outputFilename = basename(outputSwaggers[0]);
135+
return extraSwaggerSuppressions.length > 0;
136+
},
137+
);
84138

85-
stdOutput += "\nOutput folder:\n";
86-
stdOutput += outputFolder + "\n";
139+
stdOutput += `\nSwaggers excluded via suppressions.yaml:\n`;
140+
stdOutput += suppressedSwaggers.join("\n") + "\n";
87141

88-
// Filter to only specs matching the folder and filename extracted from the first output-file.
89-
// Necessary to handle multi-project specs like keyvault.
90-
//
91-
// Globby only accepts patterns like posix paths.
92-
const pattern = path.posix.join(...outputFolder.split(path.sep), "**", outputFilename);
93-
const allSwaggers = (await globby(pattern, { ignore: ["**/examples/**"] })).map(
94-
// Globby always returns posix paths
95-
(p) => normalize(p),
96-
);
97-
98-
// Filter to files generated by TypeSpec
99-
const tspGeneratedSwaggers = await filterAsync(
100-
allSwaggers,
101-
async (swaggerPath: string) => {
102-
const swaggerText = await readFile(swaggerPath, { encoding: "utf8" });
103-
const swaggerObj = JSON.parse(swaggerText) as {
104-
info?: Record<string, unknown>;
142+
const remainingSwaggers = tspGeneratedSwaggers.filter(
143+
(s) => !suppressedSwaggers.includes(s),
144+
);
145+
146+
stdOutput += `\nRemaining swaggers:\n`;
147+
stdOutput += remainingSwaggers.join("\n") + "\n";
148+
149+
const extraSwaggers = remainingSwaggers.filter((s) => !outputSwaggers.includes(s));
150+
151+
if (extraSwaggers.length > 0) {
152+
// Helper function to extract version from swagger path
153+
// Normalize to POSIX path for consistent pattern matching
154+
const extractVersion = (swaggerPath: string): string | null => {
155+
const posixPath = swaggerPath.split(path.sep).join(path.posix.sep);
156+
const match = posixPath.match(/\/(preview|stable)\/([^/]+)\//);
157+
return match ? match[2] : null;
105158
};
106-
return (
107-
swaggerObj["info"]?.["x-typespec-generated"] !== undefined ||
108-
swaggerObj["info"]?.["x-cadl-generated"] !== undefined
109-
);
110-
},
111-
);
112-
113-
stdOutput += `\nSwaggers matching output folder and filename:\n`;
114-
stdOutput += tspGeneratedSwaggers.join("\n") + "\n";
115-
116-
const suppressedSwaggers = await filterAsync(
117-
tspGeneratedSwaggers,
118-
async (swaggerPath: string) => {
119-
const suppressions = await getSuppressions(swaggerPath);
120-
121-
const extraSwaggerSuppressions = suppressions.filter(
122-
(s) => s.rules?.includes(this.name) && s.subRules?.includes("ExtraSwagger"),
123-
);
124-
125-
// Each path must specify a single version (without wildcards) under "preview|stable"
126-
//
127-
// Allowed: data-plane/Azure.Contoso.WidgetManager/preview/2022-11-01-preview/**/*.json
128-
// Disallowed: data-plane/Azure.Contoso.WidgetManager/preview/**/*.json
129-
// Disallowed: data-plane/**/*.json
130-
//
131-
// Include "." since a few specs use versions like "X.Y" instead of "YYYY-MM-DD"
132-
const singleVersionPattern = "/(preview|stable)/[A-Za-z0-9._-]+/";
133-
134-
for (const suppression of extraSwaggerSuppressions) {
135-
for (const p of suppression.paths) {
136-
if (!p.match(singleVersionPattern)) {
137-
throw new Error(
138-
`Invalid path '${p}'. Path must only include one version per suppression.`,
139-
);
140-
}
159+
160+
// Check if all extra swaggers are preview versions
161+
const allArePreview = extraSwaggers.every((s) => {
162+
const posixPath = s.split(path.sep).join(path.posix.sep);
163+
return posixPath.includes("/preview/");
164+
});
165+
166+
let isOnlyOlderPreviews = false;
167+
if (allArePreview) {
168+
// Get all preview versions from tspGeneratedSwaggers
169+
const previewVersions = tspGeneratedSwaggers
170+
.filter((s) => {
171+
const posixPath = s.split(path.sep).join(path.posix.sep);
172+
return posixPath.includes("/preview/");
173+
})
174+
.map(extractVersion)
175+
.filter((v): v is string => v !== null);
176+
177+
if (previewVersions.length > 0) {
178+
// Find the latest preview version (sort descending)
179+
const sortedVersions = [...new Set(previewVersions)].sort().reverse();
180+
const latestPreview = sortedVersions[0];
181+
182+
// Check if any extraSwagger is from the latest preview
183+
const hasLatestPreview = extraSwaggers.some((s) => {
184+
const version = extractVersion(s);
185+
return version === latestPreview;
186+
});
187+
188+
isOnlyOlderPreviews = !hasLatestPreview;
141189
}
142190
}
143191

144-
return extraSwaggerSuppressions.length > 0;
145-
},
146-
);
147-
148-
stdOutput += `\nSwaggers excluded via suppressions.yaml:\n`;
149-
stdOutput += suppressedSwaggers.join("\n") + "\n";
150-
151-
const remainingSwaggers = tspGeneratedSwaggers.filter(
152-
(s) => !suppressedSwaggers.includes(s),
153-
);
154-
155-
stdOutput += `\nRemaining swaggers:\n`;
156-
stdOutput += remainingSwaggers.join("\n") + "\n";
157-
158-
const extraSwaggers = remainingSwaggers.filter((s) => !outputSwaggers.includes(s));
159-
160-
if (extraSwaggers.length > 0) {
161-
// Helper function to extract version from swagger path
162-
// Normalize to POSIX path for consistent pattern matching
163-
const extractVersion = (swaggerPath: string): string | null => {
164-
const posixPath = swaggerPath.split(path.sep).join(path.posix.sep);
165-
const match = posixPath.match(/\/(preview|stable)\/([^/]+)\//);
166-
return match ? match[2] : null;
167-
};
168-
169-
// Check if all extra swaggers are preview versions
170-
const allArePreview = extraSwaggers.every((s) => {
171-
const posixPath = s.split(path.sep).join(path.posix.sep);
172-
return posixPath.includes("/preview/");
173-
});
174-
175-
let isOnlyOlderPreviews = false;
176-
if (allArePreview) {
177-
// Get all preview versions from tspGeneratedSwaggers
178-
const previewVersions = tspGeneratedSwaggers
179-
.filter((s) => {
180-
const posixPath = s.split(path.sep).join(path.posix.sep);
181-
return posixPath.includes("/preview/");
182-
})
183-
.map(extractVersion)
184-
.filter((v): v is string => v !== null);
185-
186-
if (previewVersions.length > 0) {
187-
// Find the latest preview version (sort descending)
188-
const sortedVersions = [...new Set(previewVersions)].sort().reverse();
189-
const latestPreview = sortedVersions[0];
190-
191-
// Check if any extraSwagger is from the latest preview
192-
const hasLatestPreview = extraSwaggers.some((s) => {
193-
const version = extractVersion(s);
194-
return version === latestPreview;
195-
});
196-
197-
isOnlyOlderPreviews = !hasLatestPreview;
192+
if (!isOnlyOlderPreviews) {
193+
success = false;
194+
errorOutput += pc.red(
195+
`\nOutput folder '${outputFolder}' appears to contain TypeSpec-generated ` +
196+
`swagger files, not generated from the current TypeSpec sources. ` +
197+
`Perhaps you deleted a version from your TypeSpec, but didn't delete ` +
198+
`the associated swaggers?\n\n`,
199+
);
200+
errorOutput += pc.red(extraSwaggers.join("\n") + "\n");
201+
} else {
202+
stdOutput += pc.yellow(
203+
`\nNote: Found extra preview swaggers from older versions (not the latest preview). ` +
204+
`These are allowed to remain:\n`,
205+
);
206+
stdOutput += pc.yellow(extraSwaggers.join("\n") + "\n");
198207
}
199208
}
200-
201-
if (!isOnlyOlderPreviews) {
202-
success = false;
203-
errorOutput += pc.red(
204-
`\nOutput folder '${outputFolder}' appears to contain TypeSpec-generated ` +
205-
`swagger files, not generated from the current TypeSpec sources. ` +
206-
`Perhaps you deleted a version from your TypeSpec, but didn't delete ` +
207-
`the associated swaggers?\n\n`,
208-
);
209-
errorOutput += pc.red(extraSwaggers.join("\n") + "\n");
210-
} else {
211-
stdOutput += pc.yellow(
212-
`\nNote: Found extra preview swaggers from older versions (not the latest preview). ` +
213-
`These are allowed to remain:\n`,
214-
);
215-
stdOutput += pc.yellow(extraSwaggers.join("\n") + "\n");
216-
}
217209
}
218210
} else {
219211
success = false;

0 commit comments

Comments
 (0)