* Plain JS binder errors
Issue select errors from the binder in JS files that do not have checkJS
explicitly turned on or off. These errors mirror runtime checks done by
Javascript.
* Rest of plain JS binder errors
* address PR comments
* Only issue binder errors in plain JS.
Checker errors require requesting diagnostics, which stll needs to be
peformance tested.
This commit removes one cross-file duplicate declaration error in the tests.
* fix const lint
* Always issue cannot find name did-you-mean error
This PR issues "cannot find ${name}, did you mean ${name}" errors for
identifiers and propery access expressions in JS files *without*
`// @ts-check` and without `// @ts-nocheck`. This brings some benefits of
Typescript's binder to all Javascript users, even those who haven't
opted into Typescript checking.
```js
export var inModule = 1
inmodule.toFixed() // errors on exports
function f() {
var locals = 2
locale.toFixed() // errors on locals
}
var object = {
spaaace: 3
}
object.spaaaace // error on read
object.spaace = 2 // error on write
object.fresh = 12 // OK, no spelling correction to offer
```
To disable the errors, add `// @ts-nocheck` to the file. To get the
normal checkJs experience, add `// @ts-check`.
== Why This Works ==
In a word: precision. This change has low recall — it misses lots
of correct errors that would be nice to show — but it has high
precision: almost all the errors it shows are correct. And they come
with a suggested correction.
Here are the ingredients:
1. For unchecked JS files, the compiler suppresses all errors except
two did-you-mean name resolution errors.
2. Did-you-mean spelling correction is already tuned for high
precision/low recall, and doesn't show many bogus errors even in JS.
3. For identifiers, the error is suppressed for suggestions from global files.
These are often DOM feature detection, for example.
4. For property accesses, the error is suppressed for suggestions from
other files, for the same reason.
5. For property accesses, the error is suppressed for `this` property
accesses because the compiler doesn't understand JS constructor
functions well enough.
In particular, it doesn't understand any inheritance patterns.
== Work Remaining ==
1. Code cleanup.
2. Fix a couple of failures in existing tests.
3. Suppress errors on property access suggestions from large objects.
4. Combine (3) and (4) above to suppress errors on suggestions from other, global files.
5. A little more testing on random files to make sure that precision
is good there too.
6. Have people from the regular Code editor meeting test the code and
suggest ideas.
* all (most?) tests pass
* NOW they all pass
* add tonnes of semi-colons
* restore this.x check+add a test case
* make ts-ignore/no-check codefix work in unchecked js
* Issues errors only in the language service
* add a few more tests
* fix incorrect parentheses
* More cleanup in program.ts
* Improve readability of isExcludedJSError
* make diff in program.ts smaller via closure
* Switch unchecked JS did-you-mean to suggestion
Instead of selectively letting errors through.
* undo more missed changes
* disallow ignoring suggestions
* Issue different messages for plain JS than others
Straw text for the messages, I just changed the modals to avoid name
collisions.
Previously, getExternalModuleMember passed through its received value of
`dontResolveAlias` to every function that accepted it. That includes (1)
resolution of the module symbol and (2) resolution of the module
specifier. However, in TS, the module symbol is never an alias anyway, so
dontResolveAlias doesn't make a difference. In JS, the module symbol
*can* be an alias, and it should always be resolved. That's what this PR
does.
Fixes#43713
* Allow redeclaring CommonJS alias with initial undefined
This allows us to read our own output, plus the times when people
manually write exactly the same pattern.
Fixes#40555
* Use control flow to type commonjs exports
1. Could probably use a *lot* more tests.
2. getTypeOfAlias redoes some work from resolveAlias because it needs to
not resolve the alias completely, just to its export.
* fix lint, improve jsdoc
* Add tests, improve+fix control flow
1. Update the module.exports test to match the exports ones.
2. Add a test of evolving commonjs type.
3. Add a test of assignment as last statement.
(1) exposed a bug that required a better synthetic reference. (3)
exposed a bug that was most easily fixed by giving source files a
`endFlowNode` like functions and setting it in the binder.
* fix lint
e350c357 (#40228) introduced a subtle bug: it switched the flags to an
alias, dropping `SymbolFlags.Property` --- and that makes
`symbolIsValue()` get to the `resolveAlias(symbol)` call, which leads to
`getPropertyOfType()` with`resolved.callSignatures`+`constructSignatures`
being `undefined`. So initialize them in `setStructuredTypeMembers`
before calling `getNamedMembers()`.
Fixes#42350
Fixes resolution of export aliases in the postfix-property-access case
of commonjs require:
```js
const { x } = require('./foo').nested
x
```
This program would previously fail if `x` was an export alias.
Fixes#41422
* Fix default-property-assignment decls+prototype property decls
The check in getAssignedClassSymbol forgot to allow for default-property
assignment declarations, in part because it wasn't using a utility
function to do so.
* small cleanup
* make allowDeclaration parameter required
* Fix error on duplicate commonjs exports
Previously, the code missed setting the parent pointer for the lhs
access expression.
Also add declaration emit of element access expressions, missed in my
previous PR.
* Switch to excludes=None, add test case
CommonJS exports have None excludes, but still have an error issued by
the checker. This is the previous behaviour even though it would be nice
to add some exclusions.
* Use ad-hoc code for nested destructuring in require
Nested destructuring doesn't really map to ES imports:
```js
const { utils: { nub, intercalate } } = require('./monopackage')
```
Previously, isRequireVariableDeclaration walked up binding elements
until it reached a variable declaration. This change instead only walks
up one binding element and stops. Then it's not bound as an alias and
uses the checker-only code to produce types for the nested-imported
identifiers.
Fixes#40143
* revert binder formatting change
* Alias for `module.exports.x = x`
This fixes#40155 in a surprisingly small amount of code.
* Treat any aliasable expression as an alias
* test internal references to exported class
* Passing --checkJs always sets --allowJS
Even if you have `"allowJs": false`. This is not a useful combination.
Changing this makes the compiler more friendly and easier to describe.
* only set allowjs if not explicitly set
* remove stray newline
* restore bad config error
* use an accessor function instead
The commonjs-specific code for resolving access expressions on `require`
assumes a fake commonjs export. For real exports, it needs to call
resolveSymbol since it's outside the normal alias-resolving
infrastructure.
From the user tests:
```js
const x = require('y').z.ka
```
would cause the crash because I forgot to call
getLeftMmostPropertyAccessExpression in one place.
Note that this doesn't fix the alias, it just stops the crash.
* First attempt at aliases for require
* test+initial support for const x=require
* 1st round of baseline improvements
* 2nd round of baseline updates
* support property access after require
* check @type tag on require
* forbid expando missing namespaces on aliases
taken from #39558 as soon as it was created
* accept error baselines that are good, actually
* Scribbling on d.ts emit code
* use getSpecifierForModuleSymbol
* hideous hack for module.exports of aliases
* Fix module.exports.x --> export list emit
* fix isLocalImport predicate
* require only creates aliases in JS
* re-handle json imports
* update fourslash baseline
* Cleanup in the checker
1. Simplify alias resolution.
2. Simplify variable-like checking.
3. Make binding skip require calls with type tags -- they fall back to
the old require-call code and then check from there.
I haven't started on the declaration emit code since I don't know what
is going on there nearly as well.
* Function for getting module name from require call
* First round of cleanup plus a new test
Found one missing feature, not sure it's worth adding.
* more small cleanup
* more cleanup, including lint
* use trackSymbol, not serializeTypeForDeclaration
* Code review comments, plus remove unneeded code
Ad-hoc type reference resolution for `require` isn't needed anymore.
* find all refs works
* remove old ad-hoc code
* make it clear that old behaviour is not that correct
* update api baselines
* remove outdated comment
* PR feedback
1. Fix indentation
2. Add comment for exported JSON emit
3. Add test case for nested-namespace exports.
* add a fail-case test (which passes!)
* Bind alias ThisProperty assignment declarations
This is a quick prototype that does the wrong thing at the wrong time
with the wrong technique.
* Preliminary checker handling for aliases
Duplicative and untested, but I think I updated all the places that need
updating.
* new is error; old one should not have been removed
* I don't even know what's happening with this test
* cleanup and testing in the checker
* binder: use lookupSymbolForNameWorker instead of mutable
This should have about the same behaviour and is much easier to
understand.
Also refactor common code a bit.
* Shorter name of lookupSymbolForName
Once upon a time there was a parent/worker function, but now it's just a
single function again. No need for the -Worker suffix.
* remove oodate comment
* fix switch-case-break lint
* Refactor and move functions
* Rename and improve type of getContextualTypeForAssignmentDeclaration
Previously, property assignments with `void 0` initialisers were treated
like any other values. But this causes us to choke when checking our own
commonjs emit. This is something that happens by mistake a fair amount,
so this PR goes back to treating these assignments as normal
assignments.
This should allow us to check our own emit in loose cases without
harming other code bases, since `void 0` is rarely written by hand.
Note that other errors still happen: noImplicitAny forbids
accessing undeclared properties on object literals, and strictNullChecks
forbids assigning `undefined` to properties with a different type.
However, this change is enough to unblock compilation with
`strictNullChecks: false`.
Neither `x[0]` nor `x[(0)]` should be dynamic names. Previously, the
latter was because `isDynamicName` didn't skip parentheses.
Since the binder treats dynamic names in property assignments as
assignment declarations, this incorrectly tried to create a binding for
expressions like `x[(0)] = 1`.
This caused an assert because `x[(0)]` would not take the dynamic name
code path during binding (`hasDynamicName` returned false), but the
normal code path for static names.
* Avoid circular reference in this-property assignments
To do this, don't check this-property assigments that have the
this-property of the lhs appearing somewhere on the rhs:
```js
class C {
m() {
this.x = 12
this.x = this.x + this.y
}
}
```
I tried suppressing the circularity error, but because we cache the
first type discovered for a property, this still results in an implicit
any for `x` in the previous example. It just doesn't have an error.
Fixes#35099
* Add test case + rename function
* Use isMatchingReference
* Fix crash for private identifier in expando assignments
It does this by disallowing private identifiers from expando assignments
entirely. I haven't thought of a scenario where they make sense, but I
haven't thought about it exhaustively either.
Fixes#37356
* Update baselines
I think the new error is probably better. It's certainly different!
Prevent it from using the (return) type of a function. Could also check
the condition before calling the function, but there are two places that
need the check, and OTOH, all calls check the result so returning
`undefined` should work.
(This problem was introduced in PR#32553.)
Fixes#33741.
* Ignore @private on constructor functions
This was incorrect in the best of circumstances and caused a crash when
the parent of the function had no symbol, because the accessibility
check assumed it was operating on a constructor and that the parent was
always the containing class.
* Non-constructors are always accessible
Previously, all function-like kinds were accessible, which includes
constructors. This was wrong.
Top-level this-assignments do not support computed properties. But the
binder forgets to check for computed properties and tries to bind them
normally. This hits a helpful assert.
This change stop binding this-properties with computed properties at the
top-level. There's nothing sensible we could do with them; we're unable
to late-bind entries to the global scope or to modules.
The constructor function code path in the return type checking of
signatures needs to pass the *merged* symbol of the declaration to
getDeclaredTypeOfClassOrInterface. Other callers of
getDeclaredTypeOfClassOrInterface do this, or used an already-merged
symbol.
Fixes#33993
* Fix crash in expando assignment to alias
This PR disallows expando assignments
Fixes#34493, but disallows the prototype assignment nonetheless.
* Revert mistaken changes
* Fix stack overflow in circular assignment declaration
It also needs to have multiple assignments so that it has a ValueModule
flag.
Fixes#33006
* remove errant comment
* Remove other possible circularity
* Restore fallback with additional condition
* migrate nullish coalescing commit
* add more test case
* add branch type check test
* add more tests
* fix nullish precedence
* update public api
* add rescan question question token to fix regression
* update public api baseline
* Added tests that emit for nullish coalescing operator conforming with grammar restrictions when assertions are used.
* Fixed emit to hoist temporary variables (they previously went undeclared).
Added tests to ensure calls and property accesses are only called once.
* use not equal to null
* rename factory
* add grammar check
* fix more cases
* Fix handling of nullish coalescing oprator in expando objects.
* Fixed classifier to support ?? operator.
* update baseline
* accept baseline
* fix review
* update emitter and more testcase
* update control flow
* make linter happy
* update libs
* avoid unnecessary assert
* fix typooo
* Fixes for control-flow analysis
* Start enabling element access special assignment
* Treat element access assignment as special assignment in JS
* Make declarations for bindable element access expressions
* Fix navigationBar crash
* Add multi-level test for JS
* Propagate element access expressions to more code paths
* Fix property access on `this`
* Add quick info test
* Uhhh I guess this is fine
* Fix module["exports"] and property access chained off element access
* Add test for this property assignment
* Add test for and fix prototype property assignment
* Fix teeeest???
* Update APIs
* Fix element access declarations on `this`
* Fix go-to-definition
* Add declaration emit to tests
* Reconcile with late-bound symbol element access assignment
* Fix baselines
* Add JS declaration back to tests
* Fix JS declaration emit of non-late-bound string literal property names
* Revert accidental auto-format
* Use `isAccessExpression`
* Add underscore escaping member to test
* Fix and test navBar changes
* Support some late-bound special property assignments
* Integrate PR feedback
* PR feedback
* Enable declaration on core tests
* Specialize type of binary expression used for late binding, speculative fix to navigation bar, merge check and type for elem/property accesses
* Add test showing current nav bar behavior (specifically the lack thereof)
* Fix constructor function type reference lookup
I knew I missed some code in the constructor-functions-as-classes PR.
This simplifies the type reference resolution code as well.
* Simplify and document js alias type resolution
* Cache JS inferred class type symbol
Note that many sources merge into a single target, so the *source*
[links] is the one that caches the merged target.
The reason this is a problem is not that many sources merge into a
single target, but that both getTypeOfSymbol and getDeclaredTypeOfSymbol
end up calling mergeJSSymbols with the same [source,target] pair. The
merge should not happen twice.
* Remove more verbose debug assertion message
* Fix isJSConstructor check + update baselines
* inferClassSymbol cache now track multiple targets
* Initial implementation
The original test passes but I haven't run any other tests yet, so I
assume the world is now broken.
* Append constructor function construct sigs
Instead of overwriting them
* Grab bag of improvements.
1. Mark @class-tagged functions with Class too.
2. Only gather local type parameters of constructor functions.
3. Remove getJSClassType calls with getDeclaredTypeOfSymbol.
4. Add a couple more failing tests.
getDeclaredTypeOfClassOrInterface now needs to understand prototype
assignment. That's next, I think.
* Prototype assignments work now
1. Binder marks prototype assignments as Class now.
2. Checker merges prototype assignments using the same merge code as for
functions and their declarations. No more intersections.
Many fewer failing tests now.
* Mark prototype-property assignments as Class
Even if there are no this-property assignments in them. (Then why are
you using a class?).
* Simplify getJSClassType, remove calls to its guts
It's probably not needed because now it's just a conditional call to
getDeclaredTypeOfSymbol, and I think most callers already know whether
they have a JS constructor function beforehand.
* isJSDocConstructor doesn't need to check prototype anymore
Because all the properties are merged during getDeclaredTypeOfSymbol.
* outer type parameter lookup follow prototype assignment
* this-type and -expression support in ctor funcs
Pretty cool!
* Fix remaining tests
* Fix minor lint
* Delete now-unused code
* Add class flag to nested class declarations
Also remove old TODOs
* Property assignment uses parent type annotation
First draft, will write full explanation later.
Also makes sure that jsdoc is ignored in TS. It was not before.
* Update baselines
* Fix infinite loop: module.exports alias detection
Previously, module.exports alias detection in the binder could enter an
infinite recursion. Now it does not.
Notably, there are *two* safeguards: a counter limiter that I set at
100, and an already-seen set. I actually prefer the counter limiter code
because it's foolproof and uses less memory. But it takes 100
iterations to escape from loops.
* fix space lint
* Remove already-seen map
* Restore original code from bind-toplevel-this
With one or two additional comments
* Working in JS, but the symbol is not right.
Still need to
1. Make it work in Typescript.
2. Add test (and make them work) for the other uses of GlobalThis:
window, globalThis, etc.
* Check in TS also; update some tests
Lots of tests still fail, but all but 1 change so far has been correct.
* Update baselines
A couple of tests still fail and need to be fixed.
* Handle type references to globalThis
The type reference must be `typeof globalThis`. Just `globalThis` will
be treated as a value reference in type position -- an error.
* Restore former behaviour of implicitThis errors
I left the noImplicitThis rule for captured use of global this in an
arrow function, even though technically it isn't `any` any more --
it's typeof globalThis. However, you should still use some other method
to access globals inside an arrow, because captured-global-this is super
confusing there.
* Test values with type globalThis
I ran into a problem with intersecting `Window & typeof globalThis`:
1. This adds a new index signature to Window, which is probably not
desired. In fact, with noImplicitAny, it's not desired on globalThis
either I think.
2. Adding this type requires editing TSJS-lib-generator, not this repo.
So I added the test cases and will probably update them later, when
those two problems are fixed.
* Add esnext declaration for globalThis
* Switch to symbol-based approach
I decided I didn't like the import-type-based approach.
Update baselines to reflect the difference.
* Do not suggest globals for completions at toplevel
* Add tests of element and property access
* Look up globalThis using normal resolution
globalThis is no longer constructed lazily. Its synthetic Identifier
node is also now more realistic.
* Update fourslash tests
* Add missed fourslash test update
* Remove esnext.globalthis.d.ts too
* Add chained globalThis self-lookup test
* Attempt at making globalThis readonly
In progress, had to interrupt for other work.
* Add/update tests
* Addres PR comments:
1. Add parameter to tryGetThisTypeAt to exclude globalThis.
2. Use combined Module flag instead combining them in-place.
3. SymbolDisplay doesn't print 'module globalThis' for this expressions
anymore.
Previously, the compiler would crash when binding a non-top-level
property assignment on the symbol of an unresolved module:
```js
import x from 'arglebaz'
{
x.bar = 1
}
```
That's because `x` looks like an alias but doesn't have a
valueDeclaration (since there is no file named 'arglebaz'), and the new
code for binding Object.defineProperty calls forgot to check for an
undefined valueDeclaration.
This change adds the checks for an undefined valueDeclaration.
* Do not merge commonsjs exports onto an alias
getCommonJSExportEquals merges export assignments and export property
assignments. Something like this, which has no equivalent structure in
TS:
```js
module.exports = function() { }
module.exports.expando = 1
```
However, it is sometimes called with an alias, when its
parent, resolveExternalModuleSymbol, is called with dontResolveAlias:
true, and when the initialiser of the export assignment is an alias:
```js
function alias() { }
module.exports = alias
module.exports.expando = 1
```
In this case, (1) the actual value `alias` will have already merged in a
previous call to getCommonJSExportEquals and
(2) getTypeOfSymbol will follow the alias symbol to get the right type.
So getCommonJSExportEquals should do nothing in this case.
This bug manifests in the code for dynamic imports, which calls
getTypeOfSymbol on the incorrectly merged alias, which now has enough
value flags--Function, for example--to take the wrong branch and
subsequently crash.
* Update baselines
In JS, when you assign `module.exports = exports` and the entire module is
wrapped in an IIFE, the resulting `export=` symbol, after following
aliases, is the module itself. This results in trying to merge the
file's exports with itself inside `getCommonJsExportEquals`, since it
thinks that it has found new exports, possibly in an object literal,
that need to be merged with the file's exports.
For example:
```js
(function() {
exports.a = 1
module.exports = exports
})()
```
