Skip to content

Conflicting routes #7856

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
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
3 changes: 2 additions & 1 deletion packages/http/src/linter.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { defineLinter } from "@typespec/compiler";
import { conflictingRouteRule } from "./rules/conflicting-route.rule.js";
import { opReferenceContainerRouteRule } from "./rules/op-reference-container-route.js";

export const $linter = defineLinter({
rules: [opReferenceContainerRouteRule],
rules: [opReferenceContainerRouteRule, conflictingRouteRule],
});
214 changes: 214 additions & 0 deletions packages/http/src/rules/conflicting-route.rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
import { createRule, paramMessage } from "@typespec/compiler";
import { getAllHttpServices } from "../operations.js";
import { isSharedRoute } from "../route.js";
import type { HttpOperation } from "../types.js";
import { parseUriTemplate, type UriTemplateParameter } from "../uri-template.js";

export const conflictingRouteRule = createRule({
name: "conflicting-route",
severity: "warning",
description: "Check no 2 operations without @sharedRoute have path that might be ambiguous.",
url: "https://typespec.io/docs/libraries/http/rules/conflicting-route",
messages: {
default: paramMessage`Operations have conflicting routes. These operations could match the same URLs:\n${"details"}`,
},
create(context) {
return {
root: (program) => {
const [services, _] = getAllHttpServices(program);
for (const service of services) {
const conflicts = findConflictingRoutes(
service.operations.filter((op) => !isSharedRoute(program, op.operation)),
);

for (const conflictingOps of conflicts) {
const details = conflictingOps
.map((op) => ` - ${op.operation.name} => \`${op.uriTemplate}\``)
.join("\n");
for (const operation of conflictingOps) {
context.reportDiagnostic({
target: operation.operation,
format: { details },
});
}
}
}
},
};
},
});

function findConflictingRoutes(operations: HttpOperation[]): HttpOperation[][] {
const conflicts: HttpOperation[][] = [];

// Group operations by HTTP verb first
const verbGroups = new Map<string, HttpOperation[]>();
for (const operation of operations) {
const verb = operation.verb;
const existing = verbGroups.get(verb);
if (existing) {
existing.push(operation);
} else {
verbGroups.set(verb, [operation]);
}
}

// Check for conflicts within each verb group using a route tree
for (const operations of verbGroups.values()) {
if (operations.length < 2) continue;

const routeTree = new RouteTree();

// Add each route to the tree and collect conflicts
for (const operation of operations) {
const conflictingOps = routeTree.addRoute(operation);
if (conflictingOps.length > 0) {
// Found conflict with existing routes
const allConflicted = [operation, ...conflictingOps];

// Check if any of these operations are already in a conflict group
const existingGroup = conflicts.find((group) =>
allConflicted.some((op) => group.includes(op)),
);

if (existingGroup) {
// Merge with existing group
for (const op of allConflicted) {
if (!existingGroup.includes(op)) {
existingGroup.push(op);
}
}
} else {
// Create new conflict group
conflicts.push(allConflicted);
}
}
}
}

return conflicts;
}

/** Parse all the segments of a uri template
* Contains an array of path segments(strings) and parameters (UriTemplateParameter).
*/
function parseUriTemplateSegments(uriTemplate: string): Array<string | UriTemplateParameter> {
const parsed = parseUriTemplate(uriTemplate);
const segments = parsed.segments || [];
return segments
.filter((segment) => {
if (typeof segment === "object") {
return segment.operator !== "?" && segment.operator !== "&";
}
return true;
})
.flatMap((segment): Array<string | UriTemplateParameter> => {
if (typeof segment === "object") {
return [segment];
}
return segment.split("/");
});
}

/**
* A route tree (trie) that detects conflicting routes as they are added.
* Each node represents a path segment, and conflicts occur when:
* - A parameter segment conflicts with a literal segment at the same position
* - Multiple operations end at the same leaf node
*/
class RouteTree {
private root = new RouteNode();

/**
* Adds a route to the tree and returns any conflicting operations.
*/
addRoute(operation: HttpOperation): HttpOperation[] {
const segments = parseUriTemplateSegments(operation.uriTemplate);
return this.root.addOperation(operation, segments, 0);
}
}

class RouteNode {
/** Literal children: exact string matches */
#literalChildren = new Map<string, RouteNode>();

/** Parameter child: matches any segment (only one allowed per node) */
#parameterChild: RouteNode | null = null;

/** Operations that terminate at this node */
#operations: HttpOperation[] = [];

addOperation(
operation: HttpOperation,
segments: Array<string | UriTemplateParameter>,
index: number,
): HttpOperation[] {
// If we've consumed all segments, this operation ends here
if (index >= segments.length) {
const conflicts = [...this.#operations]; // Copy existing operations as they conflict
this.#operations.push(operation);
return conflicts;
}

const segment = segments[index];
const conflicts: HttpOperation[] = [];

if (typeof segment === "string") {
if (segment.length === 0) {
// Empty segment, continue to next
return this.addOperation(operation, segments, index + 1);
}

// Check for conflict with parameter child
if (this.#parameterChild) {
// Parameter child can match this literal, so there's a conflict
conflicts.push(...this.#collectAllOperations(this.#parameterChild));
}

// Get or create literal child
let childNode = this.#literalChildren.get(segment);
if (!childNode) {
childNode = new RouteNode();
this.#literalChildren.set(segment, childNode);
}

// Continue with remaining segments
const childConflicts = childNode.addOperation(operation, segments, index + 1);
conflicts.push(...childConflicts);
} else {
// Parameter segment

// Check for conflicts with all literal children (parameter can match any literal)
for (const literalChild of this.#literalChildren.values()) {
conflicts.push(...this.#collectAllOperations(literalChild));
}

// Get or create parameter child
if (!this.#parameterChild) {
this.#parameterChild = new RouteNode();
}

// Continue with parameter child
const childConflicts = this.#parameterChild.addOperation(operation, segments, index + 1);
conflicts.push(...childConflicts);
}

return conflicts;
}

/**
* Recursively collect all operations from this node and its descendants.
*/
#collectAllOperations(node: RouteNode): HttpOperation[] {
const operations = [...node.#operations];
for (const child of node.#literalChildren.values()) {
operations.push(...this.#collectAllOperations(child));
}

if (node.#parameterChild) {
operations.push(...this.#collectAllOperations(node.#parameterChild));
}

return operations;
}
}
169 changes: 169 additions & 0 deletions packages/http/test/rules/conflicting-route.rule.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
import { createLinterRuleTester, LinterRuleTester } from "@typespec/compiler/testing";
import { beforeEach, it } from "vitest";
import { conflictingRouteRule } from "../../src/rules/conflicting-route.rule.js";
import { Tester } from "../test-host.js";

let ruleTester: LinterRuleTester;

beforeEach(async () => {
const runner = await Tester.createInstance();
ruleTester = createLinterRuleTester(runner, conflictingRouteRule, "@typespec/http");
});

it("should detect parameter vs literal conflicts", async () => {
const message = [
"Operations have conflicting routes. These operations could match the same URLs:",
" - op2 => `/foo/fixed`",
" - op1 => `/foo/{prop}`",
].join("\n");
await ruleTester
.expect(
`
@route("/foo/{prop}") op op1(prop: string): void;
@route("/foo/fixed") op op2(): void;
`,
)
.toEmitDiagnostics([
{ code: "@typespec/http/conflicting-route", message },
{ code: "@typespec/http/conflicting-route", message },
]);
});

it("detect conflict with path expansion", async () => {
await ruleTester
.expect(
`
@route("/foo{/prop}") op op1(prop: string): void;
@route("/foo/fixed") op op2(): void;
`,
)
.toEmitDiagnostics([
{ code: "@typespec/http/conflicting-route" },
{ code: "@typespec/http/conflicting-route" },
]);
});

it("should detect operations with different parameter names but same structure", async () => {
await ruleTester
.expect(
`
@route("/providers/Microsoft.Contoso/res/{fooName}/bars/{barName}")
op getBar1(fooName: string, barName: string): void;

@route("/providers/Microsoft.Contoso/res/{name}/bars/{id}")
op getBar2(name: string, id: string): void;
`,
)
.toEmitDiagnostics([
{ code: "@typespec/http/conflicting-route" },
{ code: "@typespec/http/conflicting-route" },
]);
});

it("should not flag operations with different HTTP verbs", async () => {
await ruleTester
.expect(
`
@get @route("/providers/Microsoft.Contoso/res/{fooName}")
op getFoo(fooName: string): void;

@post @route("/providers/Microsoft.Contoso/res/{name}")
op createFoo(name: string): void;
`,
)
.toBeValid();
});

it("should not flag operations with different path structures", async () => {
await ruleTester
.expect(
`
@route("/providers/Microsoft.Contoso/res/{fooName}")
op getFoo(fooName: string): void;

@route("/providers/Microsoft.Contoso/bars/{barName}")
op getBar(barName: string): void;
`,
)
.toBeValid();
});

it("should not flag operations with different literal segments", async () => {
await ruleTester
.expect(
`
@route("/foo/bar") op op1(): void;
@route("/foo/baz") op op2(): void;
`,
)
.toBeValid();
});

it("should not flag operations with the exact same path if they have @sharedRoute", async () => {
await ruleTester
.expect(
`
@sharedRoute
@route("/foo/bar") op op1(): void;

@sharedRoute
@route("/foo/bar") op op2(): void;
`,
)
.toBeValid();
});

it("should not flag operations with different number of segments", async () => {
await ruleTester
.expect(
`
@route("/foo/{id}")
op op1(id: string): void;

@route("/foo/{id}/bar")
op op2(id: string): void;
`,
)
.toBeValid();
});

it("query parameters shouldn't affect", async () => {
await ruleTester
.expect(
`
@route("/widgets/{widgetName}/analytics/current")
op op1(widgetName: string, @query version?: string): void;

@route("/widgets/{widgetName}")
op op2(widgetName: string, @query version?: string): void;
`,
)
.toBeValid();
});

it("should handle three-way conflicts", async () => {
const message = [
"Operations have conflicting routes. These operations could match the same URLs:",
" - op2 => `/api/{name}`",
" - op1 => `/api/{version}`",
" - op3 => `/api/v1`",
].join("\n");
await ruleTester
.expect(
`
@route("/api/{version}")
op op1(version: string): void;

@route("/api/{name}")
op op2(name: string): void;

@route("/api/v1")
op op3(): void;
`,
)
.toEmitDiagnostics([
{ code: "@typespec/http/conflicting-route", message },
{ code: "@typespec/http/conflicting-route", message },
{ code: "@typespec/http/conflicting-route", message },
]);
});
Loading
Loading