Give a more helpful error message for certain decorators with too many arguments #18811

DanielRosenwasser · 2017-09-28T04:04:09Z

~~Right now this only works for single-signature decorators that accept no arguments. We could improve this to cases where~~

~~there are too many arguments from the decorated entity for every signature and~~
~~the min arg count is 0 for every signature~~

We now give a more helpful error message when

a decorator receives too many arguments and
0 arguments could be given

This comes up for me all the time in Angular where I end up forgetting to invoke the decorator:

@Component({ /*... */ })
class FooComponent {
  @Input foo: string; // Oops, I meant @Input(), not @Input
}

…ressions that take no arguments.

mhegazy · 2017-10-02T19:13:17Z

tests/baselines/reference/decoratorOnClassMethod6.errors.txt

@@ -1,4 +1,4 @@
-tests/cases/conformance/decorators/class/method/decoratorOnClassMethod6.ts(4,5): error TS1241: Unable to resolve signature of method decorator when called as an expression.
+tests/cases/conformance/decorators/class/method/decoratorOnClassMethod6.ts(4,5): error TS1329: This value has type '() => <T>(target: any, propertyKey: string, descriptor: TypedPropertyDescriptor<T>) => TypedPropertyDescriptor<T>' which accepts too few arguments to be used as a decorator here. Did you mean to call it first?


Would be nicer if you had the decorator name in the error message, e.g.:
Did you mean '@dec()' instead?

mhegazy · 2017-10-02T19:15:26Z

src/compiler/checker.ts

+         * In those cases, a user may have meant to *call* the expression before using it as a decorator.
+         */
+        function isPotentiallyUncalledDecorator(decorator: Decorator, signatures: Signature[]) {
+            return signatures.length && every(signatures, signature =>


should you also check that the return type is callable/any to ensure your suggestion of calling it is warranted.

any is accounted for - it's an uncalled function call. It also has no signatures.

mhegazy · 2017-10-02T19:16:47Z

This should be marked as breaking change. the recommendation is to add rest args in the signature.

mhegazy · 2017-10-02T19:22:15Z

src/compiler/checker.ts

@@ -16381,6 +16381,11 @@ namespace ts {
                return resolveUntypedCall(node);
            }

+            if (isPotentiallyUncalledDecorator(node, callSignatures)) {
+                error(node, Diagnostics.This_value_has_type_0_which_accepts_too_few_arguments_to_be_used_as_a_decorator_here_Did_you_mean_to_call_it_first, typeToString(funcType));


The type here seems weird. it might be long, and hides the main message. why not just say '${0}' expects too few arguments to be called as a decorator, did you mean @${0}()' instead?`

I think this should get the majority of the cases, for other ones we will fallback to unable to resolve decorator when called as a function message.

DanielRosenwasser · 2017-10-03T07:28:30Z

This should be marked as breaking change. the recommendation is to add rest args in the signature.

Why? These decorators could never have succeeded as they would receive too many arguments.

DanielRosenwasser added 3 commits September 27, 2017 20:46

Give a more helpful error message when users try decorating using exp…

b029857

…ressions that take no arguments.

Accepted baselines.

fbbf3d2

Use a better check.

966f370

DanielRosenwasser force-pushed the uncalledDecorator branch from 1f1f1be to 25e9e06 Compare September 30, 2017 00:02

Accepted baselines.

08ef6e4

DanielRosenwasser force-pushed the uncalledDecorator branch from 25e9e06 to 08ef6e4 Compare September 30, 2017 00:08

DanielRosenwasser added 2 commits September 29, 2017 22:01

Added test and adjusted reporting logic.

86315ed

Accepted baselines.

803b566

DanielRosenwasser changed the title ~~Give a more helpful error message for nullary-parameter decorators~~ Give a more helpful error message for certain decorators with too many arguments Oct 2, 2017

mhegazy reviewed Oct 2, 2017

View reviewed changes

DanielRosenwasser added 4 commits October 2, 2017 17:59

Improve error message for uncalled decorators.

96bb796

Added a test for an 'any'-type decorator.

392cd61

Use 'getTextOfNode'.

35cfcff

Accepted baselines.

7750a88

DanielRosenwasser force-pushed the uncalledDecorator branch from 5450bc7 to 7750a88 Compare October 3, 2017 06:28

DanielRosenwasser merged commit 9244604 into master Oct 5, 2017

DanielRosenwasser mentioned this pull request Oct 5, 2017

Quick fix for potentially uncalled decorators #18969

Merged

DanielRosenwasser deleted the uncalledDecorator branch January 15, 2018 20:05

microsoft locked and limited conversation to collaborators Jul 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give a more helpful error message for certain decorators with too many arguments #18811

Give a more helpful error message for certain decorators with too many arguments #18811

DanielRosenwasser commented Sep 28, 2017 •

edited

Loading

mhegazy Oct 2, 2017

mhegazy Oct 2, 2017

DanielRosenwasser Oct 3, 2017

mhegazy commented Oct 2, 2017

mhegazy Oct 2, 2017

DanielRosenwasser commented Oct 3, 2017 •

edited

Loading

		@@ -1,4 +1,4 @@
		tests/cases/conformance/decorators/class/method/decoratorOnClassMethod6.ts(4,5): error TS1241: Unable to resolve signature of method decorator when called as an expression.
		tests/cases/conformance/decorators/class/method/decoratorOnClassMethod6.ts(4,5): error TS1329: This value has type '() => <T>(target: any, propertyKey: string, descriptor: TypedPropertyDescriptor<T>) => TypedPropertyDescriptor<T>' which accepts too few arguments to be used as a decorator here. Did you mean to call it first?

Give a more helpful error message for certain decorators with too many arguments #18811

Give a more helpful error message for certain decorators with too many arguments #18811

Conversation

DanielRosenwasser commented Sep 28, 2017 • edited Loading

mhegazy Oct 2, 2017

Choose a reason for hiding this comment

mhegazy Oct 2, 2017

Choose a reason for hiding this comment

DanielRosenwasser Oct 3, 2017

Choose a reason for hiding this comment

mhegazy commented Oct 2, 2017

mhegazy Oct 2, 2017

Choose a reason for hiding this comment

DanielRosenwasser commented Oct 3, 2017 • edited Loading

DanielRosenwasser commented Sep 28, 2017 •

edited

Loading

DanielRosenwasser commented Oct 3, 2017 •

edited

Loading