google / google/closure-compiler

Error messages involving scoped classes will be imprecise

Open
#2,982 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
7.7k
Forks
1.2k
Avg merge
2d 12h
Merged PRs (30d)
6

Description

This is actually impossible to reproduce just yet, because it's hidden by a more severe bug that's soon to be fixed.

For now, see [the following example][1]:

```js
/** @constructor */
function Foo() {};
/** @const {!Foo} */
var outerFoo = new Foo();

function f() {
/** @constructor */
function Foo() {};
Foo.prototype.baz = function() {};

/** @const {!Foo} */
var innerFoo = outerFoo; // no error b/c same name

outerFoo.baz(); // error b/c outer Foo does not have prop
innerFoo.baz(); // no error b/c inner Foo has prop
}
```

This snippet defines two classes `Foo` - an outer one and an inner one. The JSDoc is scoped correctly: the inner reference to `Foo` does in fact refer to the inner type, as evidenced by the error on `outerFoo.baz()` but not on `innerFoo.baz()`. Currently there's a missing error on the `innerFoo = outerFoo` assignment, due to a bug where we consider types equal if they have the same name, even after they're resolved. But once that bug is fixed, this will produce a bad error message, along the lines of

```
found : Foo
required: Foo
```

which is not particularly helpful. We should probably change the error message representation of the inner `Foo` to something along the lines of `` to avoid this confusion.

In NTI, we did conditional disambiguation of template types, since it treated templates as unique types - so rather than `found: T, required: T` it would print `found: T#1, required: T#2`, but only if it encountered two unequal types that stringified the same way. We could consider doing something along those lines here, where we detect that the error message would be nonsense and only conditionally munge the name in the message.

[1]: https://closure-compiler-debugger.appspot.com/#input0%3D%252F**%2520%2540constructor%2520*%252F%250Afunction%2520Foo()%2520%257B%257D%253B%250A%252F**%2520%2540const%2520%257B!Foo%257D%2520*%252F%250Avar%2520outerFoo%2520%253D%2520new%2520Foo()%253B%250A%250Afunction%2520f()%2520%257B%250A%2520%2520%252F**%2520%2540constructor%2520*%252F%250A%2520%2520function%2520Foo()%2520%257B%257D%253B%250A%2520%2520Foo.prototype.baz%2520%253D%2520function()%2520%257B%257D%253B%250A%250A%2520%2520%252F**%2520%2540const%2520%257B!Foo%257D%2520*%252F%250A%2520%2520var%2520innerFoo%2520%253D%2520outerFoo%253B%2520%2520%252F%252F%2520no%2520error%2520b%252Fc%2520same%2520name%250A%250A%2520%2520outerFoo.baz()%253B%2520%252F%252F%2520error%2520b%252Fc%2520outer%2520Foo%2520does%2520not%2520have%2520prop%250A%2520%2520innerFoo.baz()%253B%2520%252F%252F%2520no%2520error%2520b%252Fc%2520inner%2520Foo%2520has%2520prop%250A%257D%250A%26input1%26conformanceConfig%26externs%26refasterjs-template%26CHECK_TYPES%3Dtrue%26STRICT_CHECK_TYPES%3Dtrue%26MISSING_PROPERTIES%3Dtrue%26TRANSPILE%3Dtrue%26PRESERVE_TYPE_ANNOTATIONS%3Dtrue%26PRETTY_PRINT%3Dtrue

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.