Skip to content

Commit

Permalink
Fix application of @param on child operations (#3517)
Browse files Browse the repository at this point in the history
fix #2233
  • Loading branch information
timotheeguerin authored Jun 4, 2024
1 parent deedcbb commit 24f81bf
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 10 deletions.
8 changes: 8 additions & 0 deletions .chronus/changes/param-override-spread-2024-5-4-20-52-44.md
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/compiler"
---

Fix application of `@param` doc tag on operation create with `op is` to override upstream doc
25 changes: 21 additions & 4 deletions packages/compiler/src/core/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ export function createChecker(program: Program): Checker {
const stdTypes: Partial<StdTypes> = {};
const symbolLinks = new Map<number, SymbolLinks>();
const mergedSymbols = new Map<Sym, Sym>();
const docFromCommentForSym = new Map<Sym, string>();
const augmentDecoratorsForSym = new Map<Sym, AugmentDecoratorStatementNode[]>();
const augmentedSymbolTables = new Map<SymbolTable, SymbolTable>();
const referenceSymCache = new WeakMap<
Expand Down Expand Up @@ -2494,16 +2495,27 @@ export function createChecker(program: Program): Checker {
const name = node.id.sv;
let decorators: DecoratorApplication[] = [];

const parameterModelSym = getOrCreateAugmentedSymbolTable(symbol!.metatypeMembers!).get(
"parameters"
);

if (parameterModelSym?.members) {
const members = getOrCreateAugmentedSymbolTable(parameterModelSym.members);
for (const [name, memberSym] of members) {
const doc = extractParamDoc(node, name);
if (doc) {
docFromCommentForSym.set(memberSym, doc);
}
}
}

// Is this a definition or reference?
let parameters: Model, returnType: Type, sourceOperation: Operation | undefined;
if (node.signature.kind === SyntaxKind.OperationSignatureReference) {
// Attempt to resolve the operation
const baseOperation = checkOperationIs(node, node.signature.baseOperation, mapper);
if (baseOperation) {
sourceOperation = baseOperation;
const parameterModelSym = getOrCreateAugmentedSymbolTable(symbol!.metatypeMembers!).get(
"parameters"
)!;
// Reference the same return type and create the parameters type
const clone = initializeClone(baseOperation.parameters, {
properties: createRekeyableMap(),
Expand All @@ -2512,7 +2524,7 @@ export function createChecker(program: Program): Checker {
clone.properties = createRekeyableMap(
Array.from(baseOperation.parameters.properties.entries()).map(([key, prop]) => [
key,
cloneTypeForSymbol(getMemberSymbol(parameterModelSym, prop.name)!, prop, {
cloneTypeForSymbol(getMemberSymbol(parameterModelSym!, prop.name)!, prop, {
model: clone,
sourceProperty: prop,
}),
Expand Down Expand Up @@ -5574,6 +5586,7 @@ export function createChecker(program: Program): Checker {
if (returnTypesDocs.errors) {
decorators.unshift(createDocFromCommentDecorator("errors", returnTypesDocs.errors));
}
} else if (targetType.kind === "ModelProperty") {
}
return decorators;
}
Expand Down Expand Up @@ -6513,6 +6526,10 @@ export function createChecker(program: Program): Checker {
): T {
let clone = initializeClone(type, additionalProps);
if ("decorators" in clone) {
const docComment = docFromCommentForSym.get(sym);
if (docComment) {
clone.decorators.push(createDocFromCommentDecorator("self", docComment));
}
for (const dec of checkAugmentDecorators(sym, clone, undefined)) {
clone.decorators.push(dec);
}
Expand Down
118 changes: 112 additions & 6 deletions packages/compiler/test/checker/doc-comment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import { Model, Operation } from "../../src/core/index.js";
import { getDoc, getErrorsDoc, getReturnsDoc } from "../../src/lib/decorators.js";
import { BasicTestRunner, createTestRunner } from "../../src/testing/index.js";

describe("compiler: checker: doc comments", () => {
let runner: BasicTestRunner;
beforeEach(async () => {
runner = await createTestRunner();
});
let runner: BasicTestRunner;
beforeEach(async () => {
runner = await createTestRunner();
});

describe("compiler: checker: doc comments", () => {
const expectedMainDoc = "This is a doc comment.";
const docComment = `/**
* ${expectedMainDoc}
Expand Down Expand Up @@ -238,8 +238,114 @@ describe("compiler: checker: doc comments", () => {
strictEqual(getErrorsDoc(runner.program, test), "Another string");
});
});
});

describe("@param", () => {
async function getDocForParam(code: string): Promise<string | undefined> {
const { target } = (await runner.compile(code)) as { target: Operation };
ok(target, `Make sure to have @test("target") in code.`);
return getDoc(runner.program, target.parameters.properties.get("one")!);
}

it("applies doc on param", async () => {
const doc = await getDocForParam(`
/**
* @param one Doc comment
*/
@test("target") op base(one: string): void;
`);
strictEqual(doc, "Doc comment");
});

it("@doc on param wins", async () => {
const doc = await getDocForParam(`
/**
* @param one Doc comment
*/
@test("target") op base(@doc("Explicit") one: string): void;
`);
strictEqual(doc, "Explicit");
});

it("augment @@doc on param wins", async () => {
const doc = await getDocForParam(`
/**
* @param one Doc comment
*/
@test("target") op base(one: string): void;
@@doc(base::parameters.one, "Override");
`);
strictEqual(doc, "Override");
});

it("carry over with op is", async () => {
const doc = await getDocForParam(`
/**
* @param one Doc comment
*/
op base(one: string): void;
@test("target") op child is base;
`);
strictEqual(doc, "Doc comment");
});

it("@param on child operation override parent @param", async () => {
const doc = await getDocForParam(`
/**
* @param one Doc comment
*/
op base(one: string): void;
/**
* @param one Override for child
*/
@test("target") op child is base;
`);
strictEqual(doc, "Override for child");
});

it("augment @@doc wins over @param on child operation", async () => {
const doc = await getDocForParam(`
/**
* @param one Doc comment
*/
op base(one: string): void;
/**
* @param one Override for child
*/
@test("target") op child is base;
@@doc(child::parameters.one, "Override for child again");
`);
strictEqual(doc, "Override for child again");
});

it("spread model without @param keeps doc on property", async () => {
const doc = await getDocForParam(`
model A {
@doc("Via model") one: string
}
@test("target") op base(...A): void;
`);
strictEqual(doc, "Via model");
});

it("@param override doc set from spread model", async () => {
const doc = await getDocForParam(`
model A {
@doc("Via model") one: string
}
/**
* @param one Doc comment
*/
@test("target") op base(...A): void;
`);
strictEqual(doc, "Doc comment");
});

it("using @param in doc comment of operation applies doc on the parameters", async () => {
it("applies to distinct parameters", async () => {
// One @param has a hyphen but the other does not (should handle both cases)
const { addUser } = (await runner.compile(`
Expand Down

0 comments on commit 24f81bf

Please sign in to comment.