Skip to content

Commit

Permalink
Improving diagnostics provider (#3)
Browse files Browse the repository at this point in the history
* Improving diagnostics provider

  - Specific error messages
  - Check illegal assigns or equations
  - Recommend to not use equations in match
  - Recommend to not use matchcontinue
  - Report missing else
  - Check start and end identifier match

* Update demo image
  • Loading branch information
AnHeuermann authored Apr 22, 2024
1 parent 7a61a7e commit 31aa90b
Show file tree
Hide file tree
Showing 5 changed files with 176 additions and 25 deletions.
Binary file modified images/problemMatching.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
34 changes: 22 additions & 12 deletions server/src/analyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,28 @@ type AnalyzedDocument = {
}

export class MetaModelicaQueries {
public identifierQuery: Parser.Query;
public classTypeQuery: Parser.Query;
public errorQuery: Parser.Query;
public illegalEqualsQuery: Parser.Query;
public illegalAssignQuery: Parser.Query;
public identifier: Parser.Query;
public classType: Parser.Query;
public error: Parser.Query;
public illegalEquals: Parser.Query;
public illegalAssign: Parser.Query;
public modifierAssign: Parser.Query;
public caseEquation: Parser.Query;
public missingElseCaseMatch: Parser.Query;
public matchcontinue: Parser.Query;
public startEndIdent: Parser.Query;

constructor(language: Parser.Language) {
this.identifierQuery = language.query('(IDENT) @identifier');
this.classTypeQuery = language.query('(class_type) @type');
this.errorQuery = language.query('(ERROR) @error');
this.illegalEqualsQuery = language.query('(assign_clause_a (simple_expression) (EQUALS) @error )');
this.illegalAssignQuery = language.query('[ ( equation (simple_expression) (ASSIGN) @error ) ( constraint (simple_expression) (ASSIGN) @error ) ]');
this.identifier = language.query('(IDENT) @identifier');
this.classType = language.query('(class_type) @type');
this.error = language.query('(ERROR) @error');
this.illegalEquals = language.query('(assign_clause_a (simple_expression) (EQUALS) @error )');
this.illegalAssign = language.query('[ ( equation (simple_expression) (ASSIGN) @error ) ( constraint (simple_expression) (ASSIGN) @error ) ]');
this.modifierAssign = language.query('(modification (ASSIGN) @warning)');
this.caseEquation = language.query('(onecase (EQUATION) @info)');
this.missingElseCaseMatch = language.query('(match_expression [(MATCH) (MATCHCONTINUE)] @info (cases case: (onecase)* case: (onecase) . ))');
this.matchcontinue = language.query('(match_expression . (MATCHCONTINUE) @info (expression) )');
this.startEndIdent = language.query('(class_specifier identifier: (identifier) @start endIdentifier: (identifier) @end )');
}

/**
Expand All @@ -76,7 +86,7 @@ export class MetaModelicaQueries {
* @returns Identifier
*/
public getIdentifier(node: Parser.SyntaxNode): string | undefined {
const captures = this.identifierQuery.captures(node);
const captures = this.identifier.captures(node);
if (captures.length > 0) {
return captures[0].node.text;
} else {
Expand All @@ -91,7 +101,7 @@ export class MetaModelicaQueries {
* @returns Class type
*/
public getClassType(node: Parser.SyntaxNode): string | undefined {
const captures = this.classTypeQuery.captures(node);
const captures = this.classType.captures(node);
if (captures.length > 0) {
return captures[0].node.text;
} else {
Expand Down
2 changes: 1 addition & 1 deletion server/src/util/declarations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export function nodeToDocumentSymbol(node: Parser.SyntaxNode, queries: MetaModel
const kind = getKind(node, queries) || LSP.SymbolKind.Variable;

const range = TreeSitterUtil.range(node);
const selectionRange = TreeSitterUtil.range(queries.identifierQuery.captures(node)[0].node) || range;
const selectionRange = TreeSitterUtil.range(queries.identifier.captures(node)[0].node) || range;

// Walk tree to find next class_definition
const cursor = node.walk();
Expand Down
69 changes: 65 additions & 4 deletions server/src/util/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ import * as Parser from 'web-tree-sitter';

import { MetaModelicaQueries } from './../analyzer';
import * as TreeSitterUtil from './tree-sitter';
import { logger } from './logger';

export function getDiagnosticsFromTree(tree: Parser.Tree, queries: MetaModelicaQueries): LSP.Diagnostic[] {
const diagnostics: LSP.Diagnostic[] = [];
let captures: Parser.QueryCapture[];

// Handle all ERROR nodes from tree
captures = queries.errorQuery.captures(tree.rootNode);
captures = queries.error.captures(tree.rootNode);
if (captures.length > 0) {
for (const capture of captures ) {
diagnostics.push(nodeToDiagnostic(
Expand All @@ -55,7 +56,7 @@ export function getDiagnosticsFromTree(tree: Parser.Tree, queries: MetaModelicaQ
}

// Handle all illegal equations
captures = queries.illegalEqualsQuery.captures(tree.rootNode);
captures = queries.illegalEquals.captures(tree.rootNode);
if (captures.length > 0) {
for (const capture of captures ) {
diagnostics.push(nodeToDiagnostic(
Expand All @@ -65,7 +66,7 @@ export function getDiagnosticsFromTree(tree: Parser.Tree, queries: MetaModelicaQ
}
}
// Handle all illegal assigns
captures = queries.illegalAssignQuery.captures(tree.rootNode);
captures = queries.illegalAssign.captures(tree.rootNode);
if (captures.length > 0) {
for (const capture of captures ) {
diagnostics.push(nodeToDiagnostic(
Expand All @@ -74,10 +75,70 @@ export function getDiagnosticsFromTree(tree: Parser.Tree, queries: MetaModelicaQ
"Parse error: Equations can not contain assignments (':='), use equations ('=') instead."));
}
}
captures = queries.modifierAssign.captures(tree.rootNode);
if (captures.length > 0) {
for (const capture of captures ) {
diagnostics.push(nodeToDiagnostic(
capture.node,
LSP.DiagnosticSeverity.Warning,
"Parse error: ':=' in modifiers has been deprecated, use '=' instead."));
}
}

// Inform about missing else case in match
captures = queries.missingElseCaseMatch.captures(tree.rootNode);
if (captures.length > 0) {
for (const capture of captures ) {
diagnostics.push(nodeToDiagnostic(
capture.node,
LSP.DiagnosticSeverity.Information,
"Missing else case."));
}
}

// Inform about equations in match case
captures = queries.caseEquation.captures(tree.rootNode);
if (captures.length > 0) {
for (const capture of captures ) {
diagnostics.push(nodeToDiagnostic(
capture.node,
LSP.DiagnosticSeverity.Information,
"Use 'algorithm' instead of 'equation' inside match cases."));
}
}

// Inform that matchcontinue sucks
captures = queries.matchcontinue.captures(tree.rootNode);
if (captures.length > 0) {
for (const capture of captures ) {
diagnostics.push(nodeToDiagnostic(
capture.node,
LSP.DiagnosticSeverity.Information,
"'matchcontinue' is inefficient, use 'match' and 'try-catch' instead."));
}
}

// Check start and end identifier matching
const matches = queries.startEndIdent.matches(tree.rootNode);
logger.debug(queries.startEndIdent.captureNames.toString());
if (matches.length > 0) {
for (const match of matches ) {

// Get all MISSING from tree
logger.debug("pattern: " + match.pattern.toString());

const startNode = match.captures[0].node;
const startIdent = startNode.text;
const endNode = match.captures[1].node;
const endIdent = endNode.text;

if (startIdent !== endIdent) {
diagnostics.push(nodeToDiagnostic(
endNode,
LSP.DiagnosticSeverity.Error,
`Parse error: Start and end identifier don't match.\nReplace '${endIdent}' with '${startIdent}'.`));
}
}
}

return diagnostics;
}
Expand Down
96 changes: 88 additions & 8 deletions server/src/util/test/diagnostics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,23 +43,28 @@ import { getDiagnosticsFromTree } from '../diagnostics';
const metaModelicaTestString = `
function foo
input Boolean inKey;
output Boolean res;
output Boolean res1;
output Boolean res2;
algorithm
res := match(inKey)
res1 := match (inKey)
local
case (true) equation
true := realEq(factor1, factor2);
true = intEq(i1, j1);
then true;
case (false) algorithm
case (_) algorithm
true := realEq(factor1, factor2);
true = intEq(i1, j1);
then false;
end match;
res2 := matchcontinue(inKey)
case (true) equation
then true;
else false;
end match;
end foo;
end matchcontinue;
end bar;
`;

const expectedDiagnostics: LSP.Diagnostic[] = [
Expand All @@ -68,11 +73,11 @@ const expectedDiagnostics: LSP.Diagnostic[] = [
range: {
end: {
character: 12,
line: 14
line: 15
},
start: {
character: 11,
line: 14
line: 15
}
},
severity: 1,
Expand All @@ -83,13 +88,88 @@ const expectedDiagnostics: LSP.Diagnostic[] = [
range: {
end: {
character: 13,
line: 8
line: 9
},
start: {
character: 11,
line: 9
}
},
severity: 1,
source: "MetaModelica-language-server"
},
{
message: "Missing else case.",
range: {
end: {
character: 15,
line: 6
},
start: {
character: 10,
line: 6
}
},
severity: 3,
source: "MetaModelica-language-server"
},
{
message: "Use 'algorithm' instead of 'equation' inside match cases.",
range: {
end: {
character: 24,
line: 8
},
start: {
character: 16,
line: 8
}
},
severity: 3,
source: "MetaModelica-language-server"
},
{
message: "Use 'algorithm' instead of 'equation' inside match cases.",
range: {
end: {
character: 24,
line: 20
},
start: {
character: 16,
line: 20
}
},
severity: 3,
source: "MetaModelica-language-server"
},
{
message: "'matchcontinue' is inefficient, use 'match' and 'try-catch' instead.",
range: {
end: {
character: 23,
line: 19
},
start: {
character: 10,
line: 19
}
},
severity: 3,
source: "MetaModelica-language-server"
},
{
message: "Parse error: Start and end identifier don't match.\nReplace 'bar' with 'foo'.",
range: {
end: {
character: 7,
line: 24
},
start: {
character: 4,
line: 24
}
},
severity: 1,
source: "MetaModelica-language-server"
}
Expand Down

0 comments on commit 31aa90b

Please sign in to comment.