Skip to content
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

Allow multiple decorators on same getter/setter #14584

Merged
merged 2 commits into from Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions changelog_unreleased/typescript/14584.md
@@ -0,0 +1,31 @@
#### Allow multiple decorators on same getter/setter (#14584 by @fisker)

<!-- prettier-ignore -->
```ts
// Input
class A {
@decorator()
get foo () {}

@decorator()
set foo (value) {}
}

// Prettier stable
SyntaxError: Decorators cannot be applied to multiple get/set accessors of the same name. (5:3)
3 | get foo () {}
4 |
> 5 | @decorator()
| ^^^^^^^^^^^^
6 | set foo (value) {}
7 | }

// Prettier main
class A {
@decorator()
get foo() {}

@decorator()
set foo(value) {}
}
```
41 changes: 10 additions & 31 deletions src/language-js/parse/postprocess/typescript.js
Expand Up @@ -57,40 +57,19 @@ function throwErrorForInvalidDecorator(node) {

const { SyntaxKind } = ts;
for (const modifier of modifiers) {
if (ts.isDecorator(modifier)) {
if (!nodeCanBeDecorated(node)) {
if (
node.kind === SyntaxKind.MethodDeclaration &&
// @ts-expect-error -- internal?
!ts.nodeIsPresent(node.body)
) {
throwErrorOnTsNode(
modifier,
"A decorator can only decorate a method implementation, not an overload."
);
} else {
throwErrorOnTsNode(modifier, "Decorators are not valid here.");
}
} else if (
node.kind === SyntaxKind.GetAccessor ||
node.kind === SyntaxKind.SetAccessor
) {
if (ts.isDecorator(modifier) && !nodeCanBeDecorated(node)) {
if (
node.kind === SyntaxKind.MethodDeclaration &&
// @ts-expect-error -- internal?
const accessors = ts.getAllAccessorDeclarations(
node.parent.members,
node
!ts.nodeIsPresent(node.body)
) {
throwErrorOnTsNode(
modifier,
"A decorator can only decorate a method implementation, not an overload."
);
if (
// @ts-expect-error -- internal?
ts.hasDecorators(accessors.firstAccessor) &&
node === accessors.secondAccessor
) {
throwErrorOnTsNode(
modifier,
"Decorators cannot be applied to multiple get/set accessors of the same name."
);
}
}

throwErrorOnTsNode(modifier, "Decorators are not valid here.");
}
}
}
Expand Down
Expand Up @@ -30,7 +30,7 @@ exports[`abstract-method.ts [typescript] format 1`] = `
5 |"
`;

exports[`accessor.ts [babel-ts] format 1`] = `
exports[`accessor.ts format 1`] = `
====================================options=====================================
parsers: ["typescript"]
printWidth: 80
Expand All @@ -56,17 +56,6 @@ class A {
================================================================================
`;

exports[`accessor.ts [typescript] format 1`] = `
"Decorators cannot be applied to multiple get/set accessors of the same name. (4:5)
2 | @foo()
3 | get a() {return 1}
> 4 | @bar()
| ^^^^^^
5 | set a(v) {}
6 | }
7 |"
`;

exports[`argument-list-preserve-line.ts format 1`] = `
====================================options=====================================
parsers: ["typescript"]
Expand Down
2 changes: 1 addition & 1 deletion tests/format/typescript/decorators/jsfmt.spec.js
@@ -1,3 +1,3 @@
run_spec(__dirname, ["typescript"], {
errors: { typescript: ["abstract-method.ts", "accessor.ts"] },
errors: { typescript: ["abstract-method.ts"] },
});