Skip to content
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

Fix don't emit shared route error when verb don't match #2948

Merged
merged 11 commits into from
Feb 28, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@typespec/http"
---

Fix don't emit shared route error when verb don't match
2 changes: 1 addition & 1 deletion packages/http/src/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export const $lib = createTypeSpecLibrary({
"shared-inconsistency": {
severity: "error",
messages: {
default: "All shared routes must agree on the value of the shared parameter.",
default: paramMessage`Route "${"verb"} ${"path"}" is used by 2 operations with different @sharedRoute values.`,
timotheeguerin marked this conversation as resolved.
Show resolved Hide resolved
},
},
"write-visibility-not-supported": {
Expand Down
59 changes: 47 additions & 12 deletions packages/http/src/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,61 @@ import { Program } from "@typespec/compiler";
import { reportDiagnostic } from "./lib.js";
import { getAllHttpServices } from "./operations.js";
import { isSharedRoute } from "./route.js";
import { HttpOperation, HttpService } from "./types.js";

export function $onValidate(program: Program) {
// Pass along any diagnostics that might be returned from the HTTP library
const [services, diagnostics] = getAllHttpServices(program);
if (diagnostics.length > 0) {
program.reportDiagnostics(diagnostics);
}
validateSharedRouteConsistency(program, services);
}

function groupHttpOperations(
operations: HttpOperation[]
): Map<string, Map<string, HttpOperation[]>> {
const paths = new Map<string, Map<string, HttpOperation[]>>();

for (const operation of operations) {
const { verb, path } = operation;
let pathOps = paths.get(path);
if (pathOps === undefined) {
pathOps = new Map<string, HttpOperation[]>();
paths.set(path, pathOps);
}
const ops = pathOps.get(verb);
if (ops === undefined) {
pathOps.set(verb, [operation]);
} else {
ops.push(operation);
}
}
return paths;
}
function validateSharedRouteConsistency(program: Program, services: HttpService[]) {
for (const service of services) {
const paths = new Map<string, boolean>();
for (const operation of service.operations) {
const path = operation.path;
const shared = isSharedRoute(program, operation.operation);
const val = paths.get(path);
if (shared && val === undefined) {
paths.set(path, shared);
} else if (val && val !== shared) {
reportDiagnostic(program, {
code: "shared-inconsistency",
target: operation.operation,
});
const paths = groupHttpOperations(service.operations);
for (const pathOps of paths.values()) {
for (const ops of pathOps.values()) {
let hasShared = false;
let hasNonShared = false;
for (const op of ops) {
if (isSharedRoute(program, op.operation)) {
hasShared = true;
} else {
hasNonShared = true;
}
}
if (hasShared && hasNonShared) {
for (const op of ops) {
reportDiagnostic(program, {
code: "shared-inconsistency",
target: op.operation,
format: { verb: op.verb, path: op.path },
});
}
}
}
}
}
Expand Down
19 changes: 18 additions & 1 deletion packages/http/test/http-decorators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ describe("http: decorators", () => {
]);
});

it("emit diagnostics when not all duplicated routes are declared shared", async () => {
it("emit diagnostics when not all duplicated routes are declared shared on each op conflicting", async () => {
const diagnostics = await runner.diagnose(`
@route("/test") @sharedRoute op test(): string;
@route("/test") @sharedRoute op test2(): string;
Expand All @@ -271,6 +271,14 @@ describe("http: decorators", () => {
code: "@typespec/http/shared-inconsistency",
message: `All shared routes must agree on the value of the shared parameter.`,
},
{
code: "@typespec/http/shared-inconsistency",
message: `All shared routes must agree on the value of the shared parameter.`,
},
{
code: "@typespec/http/shared-inconsistency",
message: `All shared routes must agree on the value of the shared parameter.`,
timotheeguerin marked this conversation as resolved.
Show resolved Hide resolved
},
]);
});

Expand All @@ -283,6 +291,15 @@ describe("http: decorators", () => {
expectDiagnosticEmpty(diagnostics);
});

it("do not emit diagnostics routes sharing path but not same verb", async () => {
const diagnostics = await runner.diagnose(`
@route("/test") @sharedRoute op test(): string;
@route("/test") @sharedRoute op test2(): string;
@route("/test") @post op test3(): string;
`);
expectDiagnosticEmpty(diagnostics);
});

it("emit diagnostic when wrong type for shared is provided", async () => {
const diagnostics = await runner.diagnose(`
@route("/test", {shared: "yes"}) op test(): string;
Expand Down
Loading