diff --git a/src/language-service/diagnostics.ts b/src/language-service/diagnostics.ts index 51895b5..88d6617 100644 --- a/src/language-service/diagnostics.ts +++ b/src/language-service/diagnostics.ts @@ -1,6 +1,9 @@ /** * Diagnostics module for the language service. - * Provides function argument count validation. + * Provides function argument count validation and syntax error detection. + * + * This module leverages the existing parser infrastructure for error detection, + * avoiding duplication of tokenization and parsing logic. */ import { @@ -16,6 +19,13 @@ import { DiagnosticSeverity } from 'vscode-languageserver-types'; import type { TextDocument } from 'vscode-languageserver-textdocument'; import type { GetDiagnosticsParams, ArityInfo } from './language-service.types'; import { FunctionDetails } from './language-service.models'; +import { ParseError } from '../types/errors'; + +/** + * Length of the error highlight range when position is known but token length is not. + * Used to visually indicate the location of an error in the source text. + */ +const ERROR_HIGHLIGHT_LENGTH = 10; /** * Represents a token with its position in the source text. @@ -332,3 +342,62 @@ export function getDiagnosticsForDocument( return diagnostics; } + +/** + * Creates a diagnostic from a ParseError. + * This function converts errors thrown by the parser/tokenizer into diagnostics + * that can be displayed to the user. + */ +export function createDiagnosticFromParseError( + textDocument: TextDocument, + error: ParseError +): Diagnostic { + const position = error.context.position; + let startOffset = 0; + let endOffset = textDocument.getText().length; + + if (position) { + // Convert line/column to offset + startOffset = textDocument.offsetAt({ + line: position.line - 1, // ParseError uses 1-based line numbers + character: position.column - 1 // ParseError uses 1-based column numbers + }); + // Highlight a fixed-length region from the error position + endOffset = Math.min(startOffset + ERROR_HIGHLIGHT_LENGTH, textDocument.getText().length); + } + + const range: Range = { + start: textDocument.positionAt(startOffset), + end: textDocument.positionAt(endOffset) + }; + + return { + range, + severity: DiagnosticSeverity.Error, + message: error.message, + source: 'expr-eval' + }; +} + +/** + * Creates a diagnostic from a generic Error. + * This function handles errors thrown by the parser that are not ParseError instances. + * Since these errors don't have position information, the diagnostic highlights the whole text. + */ +export function createDiagnosticFromError( + textDocument: TextDocument, + error: Error +): Diagnostic { + const text = textDocument.getText(); + const range: Range = { + start: textDocument.positionAt(0), + end: textDocument.positionAt(text.length) + }; + + return { + range, + severity: DiagnosticSeverity.Error, + message: error.message, + source: 'expr-eval' + }; +} diff --git a/src/language-service/language-service.ts b/src/language-service/language-service.ts index 01d74b3..0b7f415 100644 --- a/src/language-service/language-service.ts +++ b/src/language-service/language-service.ts @@ -37,7 +37,13 @@ import { iterateTokens } from './ls-utils'; import { pathVariableCompletions, tryVariableHoverUsingSpans } from './variable-utils'; -import { getDiagnosticsForDocument } from './diagnostics'; +import { + getDiagnosticsForDocument, + createDiagnosticFromParseError, + createDiagnosticFromError, + TokenSpan +} from './diagnostics'; +import { ParseError } from '../types/errors'; export function createLanguageService(options: LanguageServiceOptions | undefined = undefined): LanguageServiceApi { // Build a parser instance to access keywords/operators/functions/consts @@ -299,14 +305,42 @@ export function createLanguageService(options: LanguageServiceOptions | undefine /** * Analyzes the document for function calls and checks if they have the correct number of arguments. - * Returns diagnostics for function calls with incorrect argument counts. + * Returns diagnostics for function calls with incorrect argument counts, as well as + * syntax errors detected by the parser (unclosed strings, brackets, unknown characters, etc.). */ function getDiagnostics(params: GetDiagnosticsParams): Diagnostic[] { const { textDocument } = params; const text = textDocument.getText(); + const diagnostics: Diagnostic[] = []; + + // Try to parse the expression to catch syntax errors + // The parser will throw ParseError for issues like: + // - Unknown characters + // - Unclosed strings + // - Illegal escape sequences + // - Unexpected tokens + // - Missing expected tokens (like closing brackets) + try { + parser.parse(text); + } catch (error) { + if (error instanceof ParseError) { + diagnostics.push(createDiagnosticFromParseError(textDocument, error)); + } else if (error instanceof Error) { + // Handle generic errors thrown by the parser (e.g., invalid object definition) + diagnostics.push(createDiagnosticFromError(textDocument, error)); + } + } - const ts = makeTokenStream(parser, text); - const spans = iterateTokens(ts); + // Try to tokenize for function argument checking + let spans: TokenSpan[] = []; + try { + const ts = makeTokenStream(parser, text); + spans = iterateTokens(ts); + } catch (error) { + // If tokenization fails, we already have a parse error diagnostic + // Return early since we can't do function argument checking without tokens + return diagnostics; + } // Build a map from function name to FunctionDetails for quick lookup const funcDetailsMap = new Map(); @@ -314,7 +348,11 @@ export function createLanguageService(options: LanguageServiceOptions | undefine funcDetailsMap.set(func.name, func); } - return getDiagnosticsForDocument(params, spans, functionNamesSet(), funcDetailsMap); + // Get function argument count diagnostics + const functionDiagnostics = getDiagnosticsForDocument(params, spans, functionNamesSet(), funcDetailsMap); + diagnostics.push(...functionDiagnostics); + + return diagnostics; } return { diff --git a/test/language-service/language-service.ts b/test/language-service/language-service.ts index d920d97..04cb635 100644 --- a/test/language-service/language-service.ts +++ b/test/language-service/language-service.ts @@ -829,5 +829,164 @@ describe('Language Service', () => { const diagnostics = ls.getDiagnostics({ textDocument: doc }); expect(diagnostics).toEqual([]); }); + + // Extended diagnostics tests for syntax errors + // These tests verify that parser errors are properly converted to diagnostics + describe('syntax error diagnostics', () => { + it('should detect unclosed string literal with double quotes', () => { + const text = '"hello world'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser reports this as 'Unknown character' since unclosed string is not tokenized + const stringDiag = diagnostics.find(d => d.message.includes('Unknown character')); + expect(stringDiag).toBeDefined(); + expect(stringDiag?.severity).toBe(DiagnosticSeverity.Error); + }); + + it('should detect unclosed string literal with single quotes', () => { + const text = "'hello world"; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser reports this as 'Unknown character' since unclosed string is not tokenized + const stringDiag = diagnostics.find(d => d.message.includes('Unknown character')); + expect(stringDiag).toBeDefined(); + }); + + it('should detect unclosed parenthesis', () => { + const text = '(1 + 2'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser expects closing parenthesis + const parenDiag = diagnostics.find(d => d.message.includes('Expected )')); + expect(parenDiag).toBeDefined(); + }); + + it('should detect unclosed bracket', () => { + const text = '[1, 2, 3'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser reports unexpected end of input + const bracketDiag = diagnostics.find(d => d.message.includes('Unexpected token')); + expect(bracketDiag).toBeDefined(); + }); + + it('should detect unclosed brace', () => { + const text = '{a: 1, b: 2'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser reports invalid object definition + const braceDiag = diagnostics.find(d => d.message.includes('invalid object definition')); + expect(braceDiag).toBeDefined(); + }); + + it('should detect unexpected closing parenthesis', () => { + const text = '1 + 2)'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser expects EOF but found ) + const parenDiag = diagnostics.find(d => d.message.includes('Expected EOF')); + expect(parenDiag).toBeDefined(); + }); + + it('should detect unexpected closing bracket', () => { + const text = '1 + 2]'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser expects EOF but found ] + const bracketDiag = diagnostics.find(d => d.message.includes('Expected EOF')); + expect(bracketDiag).toBeDefined(); + }); + + it('should detect unexpected closing brace', () => { + const text = '1 + 2}'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser expects EOF but found } + const braceDiag = diagnostics.find(d => d.message.includes('Expected EOF')); + expect(braceDiag).toBeDefined(); + }); + + it('should detect unclosed comment', () => { + const text = '1 + /* this is a comment 2'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + // Parser reports unexpected end of input + const commentDiag = diagnostics.find(d => d.message.includes('Unexpected token')); + expect(commentDiag).toBeDefined(); + }); + + it('should detect unknown character', () => { + const text = '1 @ 2'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + const unknownCharDiag = diagnostics.find(d => d.message.includes('Unknown character')); + expect(unknownCharDiag).toBeDefined(); + }); + + it('should not report errors for valid closed strings', () => { + const text = '"hello" + "world"'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + expect(diagnostics).toEqual([]); + }); + + it('should not report errors for valid closed brackets', () => { + const text = '(1 + 2) * [3, 4][0]'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + expect(diagnostics).toEqual([]); + }); + + it('should not report errors for valid closed comments', () => { + const text = '1 + /* comment */ 2'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + expect(diagnostics).toEqual([]); + }); + + it('should handle nested brackets correctly', () => { + const text = '((1 + 2) * (3 + 4))'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + expect(diagnostics).toEqual([]); + }); + + it('should handle escaped quotes in strings', () => { + const text = '"hello \\"world\\""'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + expect(diagnostics).toEqual([]); + }); + + it('should detect syntax errors in complex expressions', () => { + const text = '(1 + "unclosed'; + const doc = TextDocument.create('file://test', 'plaintext', 1, text); + const diagnostics = ls.getDiagnostics({ textDocument: doc }); + + // Parser will report the first error it encounters (unknown character for unclosed string) + expect(diagnostics.length).toBeGreaterThanOrEqual(1); + expect(diagnostics[0].severity).toBe(DiagnosticSeverity.Error); + }); + }); }); });