Skip to content

Commit

Permalink
Allow trailing delimited in most locations (#4837)
Browse files Browse the repository at this point in the history
fix #2284 
fix #809

Extracted the parser change from pr #4372 and made it more generic to
cover also issue #809
  • Loading branch information
timotheeguerin authored Nov 21, 2024
1 parent 0664147 commit 8afd0e8
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 21 deletions.
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: feature
packages:
- "@typespec/compiler"
---

Allow trailing delimiter in array values, tuple, decorator declaration, scalar initializer, etc.
6 changes: 0 additions & 6 deletions packages/compiler/src/core/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,6 @@ const diagnostics = {
typeofTarget: "Typeof expects a value literal or value reference.",
},
},
"trailing-token": {
severity: "error",
messages: {
default: paramMessage`Trailing ${"token"}`,
},
},
"unknown-directive": {
severity: "error",
messages: {
Expand Down
15 changes: 0 additions & 15 deletions packages/compiler/src/core/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ interface ListKind {
readonly delimiter: DelimiterToken;
readonly toleratedDelimiter: DelimiterToken;
readonly toleratedDelimiterIsValid: boolean;
readonly trailingDelimiterIsValid: boolean;
readonly invalidAnnotationTarget?: string;
readonly allowedStatementKeyword: Token;
}
Expand All @@ -193,7 +192,6 @@ namespace ListKind {
const PropertiesBase = {
allowEmpty: true,
toleratedDelimiterIsValid: true,
trailingDelimiterIsValid: true,
allowedStatementKeyword: Token.None,
} as const;

Expand Down Expand Up @@ -269,7 +267,6 @@ namespace ListKind {
delimiter: Token.Comma,
toleratedDelimiter: Token.Semicolon,
toleratedDelimiterIsValid: false,
trailingDelimiterIsValid: false,
invalidAnnotationTarget: "expression",
allowedStatementKeyword: Token.None,
} as const;
Expand Down Expand Up @@ -3230,23 +3227,11 @@ function createParser(code: string | SourceFile, options: ParseOptions = {}): Pa
}

r.items.push(item);
const delimiter = token();
const delimiterPos = tokenPos();

if (parseOptionalDelimiter(kind)) {
// Delimiter found: check if it's trailing.
if (parseOptional(kind.close)) {
mutate(r.range).end = previousTokenEnd;
if (!kind.trailingDelimiterIsValid) {
error({
code: "trailing-token",
format: { token: TokenDisplay[delimiter] },
target: {
pos: delimiterPos,
end: delimiterPos + 1,
},
});
}
// It was trailing and we've consumed the close token.
break;
}
Expand Down
20 changes: 20 additions & 0 deletions packages/compiler/test/parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,18 @@ describe("compiler: parser", () => {
`scalar bar extends uuid {
init fromOther(abc: string)
}`,
`scalar bar extends uuid {
init trailingComma(abc: string, def: string,)
}`,
]);

parseErrorEach([["scalar uuid is string;", [{ message: "'{' expected." }]]]);
});

describe("operation statements", () => {
parseEach(["op foo(): int32;", "op trailingCommas(a: string,b: other,): int32;"]);
});

describe("interface statements", () => {
parseEach([
"interface Foo { }",
Expand Down Expand Up @@ -202,6 +209,7 @@ describe("compiler: parser", () => {
'namespace A { op b(param: [number, string]): [1, "hi"]; }',
"alias EmptyTuple = [];",
"model Template<T=[]> { }",
"alias TrailingComma = [1, 2,];",
]);
});

Expand Down Expand Up @@ -245,6 +253,7 @@ describe("compiler: parser", () => {
`const a = int8(123);`,
`const a = utcDateTime.fromISO("abc");`,
`const a = utcDateTime.fromISO("abc", "def");`,
`const trailingComma = utcDateTime.fromISO("abc", "def",);`,
]);
parseErrorEach([
[`const a = int8(123;`, [{ message: "')' expected." }]],
Expand All @@ -266,6 +275,7 @@ describe("compiler: parser", () => {
`const A = #["abc"];`,
`const A = #["abc", 123];`,
`const A = #["abc", 123, #{nested: true}];`,
`const Trailing = #["abc", 123,];`,
]);
});

Expand Down Expand Up @@ -814,6 +824,7 @@ describe("compiler: parser", () => {
"extern dec myDec(target: Type, optional?: StringLiteral);",
"extern dec myDec(target: Type, ...rest: StringLiteral[]);",
"extern dec myDec(target, arg1, ...rest);",
"extern dec trailingComma(target, arg1: other, arg2: string,);",
]);

parseErrorEach([
Expand Down Expand Up @@ -861,6 +872,7 @@ describe("compiler: parser", () => {
"extern fn myDec(optional?: StringLiteral): void;",
"extern fn myDec(...rest: StringLiteral[]): void;",
"extern fn myDec(arg1, ...rest): void;",
"extern fn trailingComma(arg1: other, arg2: string,);",
]);

parseErrorEach([
Expand Down Expand Up @@ -1237,6 +1249,14 @@ describe("compiler: parser", () => {
);
});

describe("template parameters", () => {
parseEach(["model Foo<T> {}", "model TrailingComma<A, B,> {}"]);
});

describe("template arguments", () => {
parseEach(["alias Test = Foo<T>;", "alias TrailingComma = Foo<A, B,>;"]);
});

describe("annotations order", () => {
function expectHasDocComment(script: TypeSpecScriptNode, content: string, index: number = 0) {
const docs = script.statements[0].docs;
Expand Down

0 comments on commit 8afd0e8

Please sign in to comment.