Skip to content

Guard optional clientInfo + fix empty filename in diagnostics for scanner errors - #167

Open
ajax16384 wants to merge 2 commits into
genericptr:trunkfrom
ajax16384:fix-clientinfo-and-scanner-diag-uri
Open

ajax16384 wants to merge 2 commits into
genericptr:trunkfrom
ajax16384:fix-clientinfo-and-scanner-diag-uri

Conversation

@ajax16384

Copy link
Copy Markdown

Diagnostics for scanner errors were published with a broken URI

Two independent fixes found while running pasls against mixed Delphi/FPC
projects, both reproduced with plain LSP clients (no editor).

1. Guard optional clientInfo in ShowConfigStatus

clientInfo is optional in the LSP spec and TClientInfo.version is not
auto-created. A minimal client that omits clientInfo crashed the server
right on initialize. Now both are checked; the client line logs as
[unspecified] when absent.

2. Empty filename in diagnostics for scanner errors

Symptom: for files whose first error comes from the scanner (e.g.
{$I ../../x.inc} that cannot be resolved → EScannerError, error code 1003),
the client never received any publishDiagnostics — the file silently looked
"clean or dead", while stderr showed the error.

Root cause: in TSourceParser.ParseSource the except branch passed the
filename through

with FParser.CurSourcePos do
  FOnError(Self, E.Message, FileName, aCode, Row, Column);

For scanner errors the parser never advanced, so CurSourcePos is still
(Row=0, Column=0, FileName='') — and its empty FileName shadowed the
filled local Filename. The diagnostic ended up in
TPublishDiagnostics.Add(fileName='') → uri = 'file:', which no client can
match to a document, so the publish was dropped.

Fixes:

  • ParseSource except: drop the with-statement, pass the local Filename
    explicitly, read Row/Column directly from CurSourcePos.
  • DoError (multi-error path): same shadowing pattern — fall back to the
    buffer filename when CurSourcePos.FileName is empty.

Result: scanner errors (unresolvable includes, etc.) are now reported with a
valid document URI instead of being silently dropped.

Related upstream fixes

While testing pasls diagnostics, two parser bugs in its dependencies were found
and fixed upstream:

clientInfo is optional in the LSP spec and TClientInfo.version is not
auto-created; accessing them without a check crashed the server on
initialize from minimal clients that omit clientInfo.
In ParseSource the except branch passed the filename through
"with FParser.CurSourcePos do", whose FileName field is empty for
scanner errors (e.g. include file not found, fired before the parser
advanced). The empty record field shadowed the filled local Filename,
so the diagnostic was published with a broken "file:" URI and clients
dropped it - the file looked like it never got any diagnostics.

- ParseSource except: drop the with-statement, pass the local Filename
- DoError: fall back to the buffer filename when CurSourcePos.FileName
  is empty (same scanner-error case)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant