v1#1484
Open
TwitchBronBron wants to merge 574 commits into
Open
Conversation
…e,tag,segment,etc, instead of looping all diagnostics
The plugin event system swallows handler exceptions and logs them via
console.error, which lets tests pass while real bugs go silent. Each error
below was firing during the test suite run; the underlying cause is fixed
in production code (or the affected test) so the ERROR log line is gone.
- util.getCallFuncType: bail when methodName is empty/missing. An unfinished
`widget@.` callfunc has tokens.methodName === undefined; the downstream
ReferenceType chain ended up in `getCacheKey('undefined')` and crashed.
- util.getAllDottedGetPartsAsString: skip undefined parts. Same input shape
pushed an undefined methodName token into the parts array; reading
part.text then crashed.
- ReferenceType (ParamTypeFromValueReferenceType.proxy.get): bail to
DynamicType when getDefaultTypeFromValueType re-wraps the same objType.
An unresolvable IntersectionType passed in returns a fresh
ParamTypeFromValueReferenceType wrapper of itself; the proxy then
forwards the get to that wrapper, which calls getDefaultTypeFromValueType
again on the same intersection, repeats forever via Reflect.get and
blows the call stack. Also skip re-wrapping in
util.getDefaultTypeFromValueType when the input is already a
ParamTypeFromValueReferenceType (defense in depth — the proxy bail
catches the live recursion, the util skip prevents future callers from
building the same chain on a single hop).
- Project.validate: bail if builder/program was disposed. ProjectManager's
Phase-2 awaited validate slot can be reached after a superseding sync
disposed the project (worker-thread or otherwise).
Test-side adjustments (not production fixes):
- Parser.spec.ts token() helper now defaults synthetic tokens to a
zero-width (0,0) location instead of `null`. Real lexer-emitted tokens
always have a location, and the parser does range arithmetic on
token.location.range for paths like splitting `exitwhile` into
`exit` + `while`. Production was correct; the helper was the gap.
- Program.spec.ts 'fires plugin events' / 'sets needsTranspiled=true':
build prepares every file in the program (including the auto-injected
bslib added by BscPlugin.beforeBuildProgram). The two test plugins
assumed `event.file` was always the authored main.brs and crashed on
bslib's AST. Guard each handler with `if (event.file !== file)`.
- LanguageServer.spec.ts 'project-activate' describe: tests fire the
event directly on the project manager without calling server.run(),
so connection.workspace was never wired up and syncLogLevel crashed.
Added a beforeEach that assigns the existing mock connection.
- ProjectManager.spec.ts 'spawns a worker thread when threading is
enabled': comment the test to note that the afterEach
manager.dispose() will print a 'MessageHandler is now disposed' error
to the console. That's expected — Phase 2 worker-thread validation
isn't awaited and the dispose tears it down before the in-flight
worker request resolves.
Splitting the file-validation forEach into three steps let cancellation land between `validateFile` and `afterValidateFile`, leaving files in a half-processed state. The next validation either re-ran BrsFileValidator (pushing duplicate symbols that CrossScopeValidator flagged as name collisions) or skipped the file entirely (no segments registered → no scope diagnostics). Collapsing the three plugin events into one per-file sequencer action makes cancellation atomic at file granularity, removing the entire half-state class of bugs without needing dedup or rollback. Plugins that need an all-files-done signal should subscribe to `afterValidateProgram` instead of `afterValidateFile`.
…ogging Adds eight regression tests covering common-file edits propagating across scopes (renames, additions, signature changes, namespace members, sync and async with cancellation, rapid overlapping edits). All eight pass under the atomic-per-file structural fix. Also adds a debug log inside ScopeValidator's `addDiagnostic` and `addMultiScopeDiagnostic` that fires when a diagnostic falls through to `ScopeValidatorDiagnosticTag.Default`. There is no `clearByFilter` for that tag anywhere in the codebase, so any diagnostic registered outside an active segment walk sticks for the lifetime of the process. This log helps identify the code paths producing those non-clearable diagnostics.
The reported diagnostics turned out to be from an unrelated tool, not from the never-cleared `Default`-tagged path. Removing the investigation log added in the previous commit.
…d compiler/LSP caches (#1705)
Contributor
Contributor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the official branch for the v1.0.0 release, tracking milesone v1.0.0.