Skip to content

Commit

Permalink
Handle hyphen in @param doc comment (#2393)
Browse files Browse the repository at this point in the history
### ℹ️ Overview

Fixes [#2390](#2390).

Strips the leading hyphen in `@param` doc comments. Preserves handling
`@param` without the hyphen.

Parser change: The hyphen in the following code now tokenizes as
`Token.Hyphen` instead of `Token.DocText`.

```
@param foo - A param doc with a leading hyphen
```

### 📝 Notes

I made the change in the parser so that the doc comment modeling only
sees the string. I used an `if` check for `"param"` that maybe could be
handled differently. I’m open to any feedback (helper function, etc), or
just to hand this off to y’all to close out at your convenience.

**Open question**: Should TypeSpec _recommend_ using the hyphen, per
[the TSDoc spec](https://tsdoc.org/pages/tags/param/)? If so, I can add
another commit(s) that updates the example specs and documentation.

### 🧪 Testing

Modified an existing unit test that happened to have two `@param` usages
already. Let me know if we should add other unit tests.

---------

Co-authored-by: Timothee Guerin <[email protected]>
Co-authored-by: Timothee Guerin <[email protected]>
  • Loading branch information
3 people authored Oct 3, 2023
1 parent c3cd099 commit 845d0d3
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@typespec/compiler",
"comment": "Handle hyphen in @param doc comment",
"type": "none"
}
],
"packageName": "@typespec/compiler"
}
28 changes: 28 additions & 0 deletions packages/compiler/src/core/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2402,6 +2402,11 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa
type ParamLikeTag = DocTemplateTagNode | DocParamTagNode;
type SimpleTag = DocReturnsTagNode | DocErrorsTagNode | DocUnknownTagNode;

/**
* Parses a documentation tag.
*
* @see <a href="https://microsoft.github.io/typespec/language-basics/documentation#tsdoc-doc-comments">TypeSpec documentation docs</a>
*/
function parseDocTag(): DocTag {
const pos = tokenPos();
parseExpected(Token.At);
Expand All @@ -2421,14 +2426,20 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa
}
}

/**
* Handles param-like documentation comment tags.
* For example, `@param` and `@template`.
*/
function parseDocParamLikeTag(
pos: number,
tagName: IdentifierNode,
kind: ParamLikeTag["kind"],
messageId: keyof CompilerDiagnostics["doc-invalid-identifier"]
): ParamLikeTag {
const name = parseDocIdentifier(messageId);
parseOptionalHyphenDocParamLikeTag();
const content = parseDocContent();

return {
kind,
tagName,
Expand All @@ -2438,6 +2449,23 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa
};
}

/**
* Handles the optional hyphen in param-like documentation comment tags.
*
* TypeSpec recommends no hyphen, but supports a hyphen to match TSDoc.
* (Original design discussion recorded in [2390].)
*
* [2390]: https://github.com/microsoft/typespec/issues/2390
*/
function parseOptionalHyphenDocParamLikeTag() {
while (parseOptional(Token.Whitespace)); // Skip whitespace
if (parseOptional(Token.Hyphen)) {
// The doc content started with a hyphen, so skip subsequent whitespace
// (The if statement already advanced past the hyphen itself.)
while (parseOptional(Token.Whitespace));
}
}

function parseDocSimpleTag(
pos: number,
tagName: IdentifierNode,
Expand Down
4 changes: 4 additions & 0 deletions packages/compiler/src/core/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ export type DocToken =
| Token.At
| Token.CloseBrace
| Token.Identifier
| Token.Hyphen
| Token.DocText
| Token.DocCodeSpan
| Token.DocCodeFenceDelimiter
Expand Down Expand Up @@ -617,6 +618,9 @@ export function createScanner(
case CharCode.Bar:
if (atConflictMarker()) return scanConflictMarker();
return next(Token.DocText);

case CharCode.Minus:
return next(Token.Hyphen);
}

if (isAsciiIdentifierStart(ch)) {
Expand Down
11 changes: 10 additions & 1 deletion packages/compiler/test/checker/doc-comment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ describe("compiler: checker: doc comments", () => {
@test("target") model Foo {}`
);

testMainDoc(
"templated model",
`${docComment}
@test("target") model Foo<T> {}
model Bar { foo: Foo<string> }`
);

testMainDoc(
"model property",
`
Expand Down Expand Up @@ -231,12 +239,13 @@ describe("compiler: checker: doc comments", () => {
});

it("using @param in doc comment of operation applies doc on the parameters", async () => {
// One @param has a hyphen but the other does not (should handle both cases)
const { addUser } = (await runner.compile(`
/**
* This is the operation doc.
* @param name This is the name param doc.
* @param age This is the age param doc.
* @param age - This is the age param doc.
*/
@test op addUser(name: string, age: string): void;
`)) as { addUser: Operation };
Expand Down
54 changes: 36 additions & 18 deletions packages/compiler/test/parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -803,37 +803,55 @@ describe("compiler: parser", () => {
*
* @param x the param
* that continues on another line
* @param y - another param
* @template T some template
* @template U - another template
* @returns something
* @pretend this an unknown tag
*/
op test<T>(x: string): string;
op test<T, U>(x: string, y: string): string;
`,
(script) => {
const docs = script.statements[0].docs;
strictEqual(docs?.length, 1);
strictEqual(docs[0].content.length, 1);

strictEqual(
docs[0].content[0].text,
"This one has a `code span` and a code fence and it spreads over\nmore than one line.\n\n```\nThis is not a @tag because we're in a code fence.\n```\n\n`This is not a @tag either because we're in a code span`."
);
strictEqual(docs[0].tags.length, 4);
strictEqual(docs[0].tags[0].kind, SyntaxKind.DocParamTag as const);
strictEqual(docs[0].tags[0].tagName.sv, "param");
strictEqual(docs[0].tags[0].paramName.sv, "x");
strictEqual(
docs[0].tags[0].content[0].text,
"the param\nthat continues on another line"
);
strictEqual(docs[0].tags[1].kind, SyntaxKind.DocTemplateTag as const);
strictEqual(docs[0].tags[1].tagName.sv, "template");
strictEqual(docs[0].tags[1].paramName.sv, "T");
strictEqual(docs[0].tags[2].kind, SyntaxKind.DocReturnsTag as const);
strictEqual(docs[0].tags[2].tagName.sv, "returns");
strictEqual(docs[0].tags[2].content[0].text, "something");
strictEqual(docs[0].tags[3].kind, SyntaxKind.DocUnknownTag as const);
strictEqual(docs[0].tags[3].tagName.sv, "pretend");
strictEqual(docs[0].tags[3].content[0].text, "this an unknown tag");
strictEqual(docs[0].tags.length, 6);
const [xParam, yParam, tTemplate, uTemplate, returns, pretend] = docs[0].tags;

strictEqual(xParam.kind, SyntaxKind.DocParamTag as const);
strictEqual(xParam.tagName.sv, "param");
strictEqual(xParam.paramName.sv, "x");
strictEqual(xParam.content[0].text, "the param\nthat continues on another line");

// `y` has hyphen in doc string, which should be dropped
strictEqual(yParam.kind, SyntaxKind.DocParamTag as const);
strictEqual(yParam.tagName.sv, "param");
strictEqual(yParam.paramName.sv, "y");
strictEqual(yParam.content[0].text, "another param");

strictEqual(tTemplate.kind, SyntaxKind.DocTemplateTag as const);
strictEqual(tTemplate.tagName.sv, "template");
strictEqual(tTemplate.paramName.sv, "T");
strictEqual(tTemplate.content[0].text, "some template");

// `U` has hyphen in doc string, which should be dropped
strictEqual(uTemplate.kind, SyntaxKind.DocTemplateTag as const);
strictEqual(uTemplate.tagName.sv, "template");
strictEqual(uTemplate.paramName.sv, "U");
strictEqual(uTemplate.content[0].text, "another template");

strictEqual(returns.kind, SyntaxKind.DocReturnsTag as const);
strictEqual(returns.tagName.sv, "returns");
strictEqual(returns.content[0].text, "something");

strictEqual(pretend.kind, SyntaxKind.DocUnknownTag as const);
strictEqual(pretend.tagName.sv, "pretend");
strictEqual(pretend.content[0].text, "this an unknown tag");
},
],
],
Expand Down

0 comments on commit 845d0d3

Please sign in to comment.