From 24f81bf39523e296a4ae8406cc919581740c3a2a Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 4 Jun 2024 15:46:58 -0700 Subject: [PATCH] Fix application of `@param` on child operations (#3517) fix #2233 --- ...param-override-spread-2024-5-4-20-52-44.md | 8 ++ packages/compiler/src/core/checker.ts | 25 +++- .../compiler/test/checker/doc-comment.test.ts | 118 +++++++++++++++++- 3 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 .chronus/changes/param-override-spread-2024-5-4-20-52-44.md diff --git a/.chronus/changes/param-override-spread-2024-5-4-20-52-44.md b/.chronus/changes/param-override-spread-2024-5-4-20-52-44.md new file mode 100644 index 0000000000..a98460705d --- /dev/null +++ b/.chronus/changes/param-override-spread-2024-5-4-20-52-44.md @@ -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 diff --git a/packages/compiler/src/core/checker.ts b/packages/compiler/src/core/checker.ts index 246d4e3e84..0e95f05fdc 100644 --- a/packages/compiler/src/core/checker.ts +++ b/packages/compiler/src/core/checker.ts @@ -349,6 +349,7 @@ export function createChecker(program: Program): Checker { const stdTypes: Partial = {}; const symbolLinks = new Map(); const mergedSymbols = new Map(); + const docFromCommentForSym = new Map(); const augmentDecoratorsForSym = new Map(); const augmentedSymbolTables = new Map(); const referenceSymCache = new WeakMap< @@ -2494,6 +2495,20 @@ 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) { @@ -2501,9 +2516,6 @@ export function createChecker(program: Program): Checker { 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(), @@ -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, }), @@ -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; } @@ -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); } diff --git a/packages/compiler/test/checker/doc-comment.test.ts b/packages/compiler/test/checker/doc-comment.test.ts index a1a385d336..57784f4baf 100644 --- a/packages/compiler/test/checker/doc-comment.test.ts +++ b/packages/compiler/test/checker/doc-comment.test.ts @@ -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} @@ -238,8 +238,114 @@ describe("compiler: checker: doc comments", () => { strictEqual(getErrorsDoc(runner.program, test), "Another string"); }); }); +}); + +describe("@param", () => { + async function getDocForParam(code: string): Promise { + 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(`