Put error spans deep on nested object literals#25140

weswigham · 2018-06-21T22:47:21Z

And include an additional location pointing back at the deeply nested property in the target type it is being assigned to. Additionally, like in classes which are incompatible with a base type, object and array literals will now flag all locations which are incompatible with the target instead of just one.

Fixes #25030 partially (for all instances involving expressions where we're tracking which properties have issues).

DanielRosenwasser · 2018-06-21T23:43:36Z

As a follow-up to today's error sync, let's use checkExpressionForMutableLocation

… at least once

weswigham · 2018-06-22T00:50:56Z

@DanielRosenwasser Done. Also fixed the enum and elaboration changes like I said I aught to be able to. Wanna review?

DanielRosenwasser · 2018-06-28T19:05:05Z

tests/baselines/reference/deeplyNestedAssignabilityIssue.errors.txt

+    [30;47m  [0m [96m~~~~~~~~~~~~~~~~~~~~~~~~~~~[0m
+    [30;47m13[0m             }
+    [30;47m  [0m [96m~~~~~~~~~~~~~[0m
+    Which is from property '"another"' of type '{ another: A; }', declared here.


Why is "another" in quotes here?

Because i'm stringifying the literal type associated with the name. It might equally be '0' or [typeof foo]. I can try to strip " from the type's string (if it's a valid identifier post-strip) if you think it'll improve the message.

DanielRosenwasser · 2018-06-28T19:05:56Z

src/compiler/diagnosticMessages.json

        "code": 6371
    },

+    "Which is from property '{0}' of type '{1}', declared here.": {


The expected type comes from property '{0}' which was declared here on type '{1}'.

or

The expected type '{0}' comes from property '{1}' which was declared here on type '{2}'.

DanielRosenwasser · 2018-06-28T19:10:04Z

src/compiler/checker.ts

+            return false;
+        }
+
+        function elaborateError(node: Expression | undefined, source: Type, target: Type): boolean {


shouldElaborateErrorOnProperty

It's not a check - it actually performs the elaboration and adds the new diagnostics - its return type merely indicates weather such an elaboration occurred.

DanielRosenwasser · 2018-06-28T20:19:42Z

src/compiler/checker.ts

+            return false;
+        }
+
+        function *generateObjectliteralElements(node: ObjectLiteralExpression): ElaborationIterator {


Capital L

DanielRosenwasser · 2018-06-28T20:22:12Z

src/compiler/diagnosticMessages.json

    },

+    "Which is from property '{0}' of type '{1}', declared here.": {
+        "category": "Message",


I thought it was "category": "Error"

As discussed duing our sync earlier, right now the category is unused for related information - we can put off deciding why they aughta be until we have a story for using them in some principled way (for, eg, elaborations vs explanations)

DanielRosenwasser · 2018-06-29T07:10:13Z

src/compiler/utilities.ts

+     *
+     * @return non-quoted string
+     */
+    export function stripQuotes(name: string) {


I feel like we should just be able to call symbolToString and get the right thing in certain cases, and just funnel along a string.

…s, and assignments

weswigham · 2018-07-02T19:30:49Z

@DanielRosenwasser I've synced with master, hopefully addressed your reviews, and added a few more tests for good measure. Have anything more you'd like to see?

DanielRosenwasser

Can you also add a test case similar to what you already had, but which uses a contextual indexed access type?

DanielRosenwasser · 2018-07-02T23:23:20Z

src/compiler/checker.ts

+            if (!length(node.properties)) return;
+            for (const prop of node.properties) {
+                if (isJsxSpreadAttribute(prop)) continue;
+                yield [prop.name, prop.initializer, getLiteralType(idText(prop.name))];


It's hard to tell what the intent is here; I'd make this an object instead of a tuple.

DanielRosenwasser · 2018-07-02T23:24:06Z

src/compiler/checker.ts

+            // Assignability failure - check each prop individually, and if that fails, fall back on the bad error span
+            let reportedError = false;
+            for (let status = iterator.next(); !status.done; status = iterator.next()) {
+                const [prop, next, nameType] = status.value;


Maybe make it clear that you're diving into next; it doesn't seem immediately obvious.

DanielRosenwasser · 2018-07-02T23:25:38Z

src/compiler/checker.ts

+                if (!isTypeAssignableTo(sourcePropType, targetPropType)) {
+                    const elaborated = next && elaborateError(next, sourcePropType, targetPropType);
+                    if (elaborated) {
+                        reportedError = true;


I think this can just become a return

No, if I were to return here, i would only issue an elaboration on the first property with an issue instead of all of them.

I could continue and un-indent and remove the else if you'd prefer that, however.

DanielRosenwasser · 2018-07-02T23:29:23Z

src/compiler/checker.ts

+        }
+
+        type ElaborationIterator = IterableIterator<[Node, Expression | undefined, Type]>;
+        function elaborateElementwise(iterator: ElaborationIterator, source: Type, target: Type) {


Can you write a quick bit about what the intuition should be before I dive into this function?

DanielRosenwasser · 2018-07-02T23:34:09Z

src/compiler/checker.ts

+            return checkTypeRelatedTo(source, target, assignableRelation, errorNode, headMessage, containingMessageChain, errorOutputObject);
+        }
+
+        function checkTypeAssignableToAndOptionallyElaborate(source: Type, target: Type, errorNode: Node | undefined, expr: Expression | undefined, headMessage?: DiagnosticMessage, containingMessageChain?: () => DiagnosticMessageChain | undefined): boolean {


Add a little blurb that this performs a structural comparison using a given expression that we can use to provide specific errors on things like properties.

weswigham · 2018-07-03T00:44:35Z

Can you also add a test case similar to what you already had, but which uses a contextual indexed access type?

Ahhh.... to what are you referring?

DanielRosenwasser · 2018-07-03T18:08:48Z

Looks like you do have a case - however, as we discussed offline, the computed property message is a little confusing given that it's not clear it you're erroring on the name.

…mbers

weswigham · 2018-07-03T18:30:19Z

@DanielRosenwasser Added; what do you think about the new error?

DanielRosenwasser · 2018-07-04T00:55:19Z

src/compiler/diagnosticMessages.json

        "category": "Error",
        "code": 2417
    },
+    "Type of computed property value is '{0}', which is not assignable to type '{1}'.": {


computed property's

DanielRosenwasser

LGTM apart from the error message suggestion

weswigham added 8 commits May 31, 2018 15:24

Add ncie deep elaborations

91d292d

Merge branch 'master' into nice-elaborations

4303c00

Nice stuff

c463bd5

Merge branch 'master' into nice-elaborations

5733e3f

Merge branch 'master' into nice-elaborations

167ec23

Modify tuple error to use length error mroe often

f832eae

Accept good baselines

36206c8

Accept meh baselines

c8fc8db

weswigham added 3 commits June 21, 2018 17:24

Fix literal types

f52c9b9

Calculate elaborations like it was the very first time again~

ea3300a

Use tristate for enum relationship to ensure elaborations are printed…

23d1e17

… at least once

Merge branch 'master' into nice-elaborations

4a757fa

DanielRosenwasser requested changes Jun 28, 2018

View reviewed changes

weswigham added 2 commits June 28, 2018 14:59

Merge branch 'master' into nice-elaborations

10b1292

Update message text, nits

e663505

DanielRosenwasser reviewed Jun 29, 2018

View reviewed changes

weswigham added 4 commits July 2, 2018 12:04

Merge branch 'master' into nice-elaborations

7b83484

move some functions back to where they were

b819165

Add test of deep JSX elaboration

1bcf91e

Add elaboration test with parenthesized expressions, comma expression…

9eb759b

…s, and assignments

weswigham mentioned this pull request Jul 2, 2018

Related error spans on "used before declared" error messages #25359

Merged

Move check to allow elaborations on more anonymous types

62ba57c

DanielRosenwasser requested changes Jul 2, 2018

View reviewed changes

Fix nits

7a344f5

Add specialized error to elaborations of nonliteral computed named-me…

dcbf33a

…mbers

DanielRosenwasser reviewed Jul 4, 2018

View reviewed changes

Update error message

ae46816

weswigham merged commit 84f5aa5 into microsoft:master Jul 4, 2018

weswigham deleted the nice-elaborations branch July 4, 2018 02:41

mhegazy mentioned this pull request Jul 16, 2018

Errors not reported on properties inside function calls #25695

Closed

ahejlsberg mentioned this pull request Jul 26, 2018

Misleading error elaboration #25971

Closed

DanielRosenwasser mentioned this pull request Jul 28, 2018

Error spans are not close enough to missing properties on deeply nested object literals #22170

Closed

ajafff mentioned this pull request Dec 9, 2018

TypeScript uses lib: es2015 to compile itself although it is supposed to run in ES5 environments #28933

Closed

microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025

typescript-bot assigned weswigham Oct 21, 2025

Conversation

weswigham commented Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanielRosenwasser commented Jun 21, 2018

Uh oh!

weswigham commented Jun 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weswigham commented Jul 2, 2018

Uh oh!

DanielRosenwasser left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weswigham Jul 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

weswigham commented Jul 3, 2018

Uh oh!

DanielRosenwasser commented Jul 3, 2018

Uh oh!

weswigham commented Jul 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanielRosenwasser left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

weswigham commented Jun 21, 2018 •

edited

Loading

weswigham Jul 3, 2018 •

edited

Loading