From ae5dd90d293d18a585d7abb66ee1b97fe158f466 Mon Sep 17 00:00:00 2001 From: Liam Barry Allan Date: Sat, 11 Feb 2023 11:20:52 -0500 Subject: [PATCH 1/4] Tests for parameter references Signed-off-by: Liam Barry Allan --- .vscode/launch.json | 3 +- server/src/language/models/cache.js | 23 +++++++- server/src/language/parser.js | 4 +- tests/suite/linter.js | 90 ++++++++++++++++++++++++++++- 4 files changed, 115 insertions(+), 5 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index d1082038..fb91e299 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -41,7 +41,8 @@ }, "env": { "INCLUDE_DIR": "${workspaceFolder}" - } + }, + "args": ["issue_204"] } ] } diff --git a/server/src/language/models/cache.js b/server/src/language/models/cache.js index cee010b4..a47f5349 100644 --- a/server/src/language/models/cache.js +++ b/server/src/language/models/cache.js @@ -16,6 +16,9 @@ export default class Cache { /** @type {Keywords} */ this.keyword = {}; + /** @type {Declaration[]} */ + this.parameters = cache.parameters || []; + /** @type {Declaration[]} */ this.subroutines = cache.subroutines || []; @@ -48,6 +51,7 @@ export default class Cache { merge(second) { if (second) { return new Cache({ + parameters: [...this.parameters, ...second.parameters], subroutines: [...this.subroutines, ...second.subroutines], procedures: [...this.procedures, ...second.procedures], variables: [...this.variables, ...second.variables], @@ -67,6 +71,7 @@ export default class Cache { getNames() { const fileStructNames = this.files.map(file => file.subItems.map(sub => sub.name)).flat(); return [ + ...this.parameters.map(def => def.name), ...this.constants.map(def => def.name), ...this.procedures.map(def => def.name), ...this.files.map(def => def.name), @@ -107,6 +112,7 @@ export default class Cache { const allStructs = [...fileStructs, ...this.structs]; const possibles = [ + ...this.parameters.filter(def => def.name.toUpperCase() === name), ...this.constants.filter(def => def.name.toUpperCase() === name), ...this.procedures.filter(def => def.name.toUpperCase() === name), ...this.files.filter(def => def.name.toUpperCase() === name), @@ -130,7 +136,7 @@ export default class Cache { } clearReferences() { - [...this.constants, ...this.files, ...this.procedures, ...this.subroutines, ...this.variables, ...this.structs].forEach(def => { + [...this.parameters, ...this.constants, ...this.files, ...this.procedures, ...this.subroutines, ...this.variables, ...this.structs].forEach(def => { def.references = []; }); @@ -166,4 +172,19 @@ export default class Cache { return globalDef; } } + + /** + * Move all procedure subItems (the paramaters) into the cache + */ + fixProcedures() { + if (this.procedures.length > 0) { + this.procedures.forEach(proc => { + if (proc.scope) { + proc.scope.parameters = [...proc.subItems]; + proc.subItems = []; + proc.scope.fixProcedures(); + } + }) + } + } } \ No newline at end of file diff --git a/server/src/language/parser.js b/server/src/language/parser.js index e405fecd..85b67ff7 100644 --- a/server/src/language/parser.js +++ b/server/src/language/parser.js @@ -1,7 +1,5 @@ /* eslint-disable no-case-declarations */ -import { parse } from "path"; - import { createBlocks, parseStatement } from "./statement"; import Cache from "./models/cache"; @@ -1211,6 +1209,8 @@ export default class Parser { scopes[0].keyword = Parser.expandKeywords(keywords); } + scopes[0].fixProcedures(); + const parsedData = scopes[0]; this.parsedCache[workingUri] = parsedData; diff --git a/tests/suite/linter.js b/tests/suite/linter.js index 9314d06f..f2db552a 100644 --- a/tests/suite/linter.js +++ b/tests/suite/linter.js @@ -3376,4 +3376,92 @@ exports.prettyCommentsChange = async () => { new Position(10, 2), ), }); -}; \ No newline at end of file +}; + +exports.issue_204 = async () => { + const lines = [ + `**free`, + ``, + `ctl-opt dftactgrp(*no);`, + ``, + `///`, + `// printf`, + `// Print to standard out`, + `// @param String value pointer`, + `///`, + `dcl-pr printf int(10) extproc('printf');`, + ` format pointer value options(*string);`, + `end-pr;`, + ``, + `dcl-ds person_t qualified template;`, + ` name int(10);`, + ` age char(50);`, + `end-ds;`, + ``, + `dcl-ds myperson likeds(person_t);`, + ``, + `myperson = PERSON_New();`, + `myperson.name = 'Liam Barry';`, + `myperson.age = 25;`, + `PERSON_printNice(myperson);`, + ``, + `return;`, + ``, + `dcl-proc PERSON_New;`, + ` dcl-pi *n LikeDS(person_t) end-pi;`, + ` // This is the constructor`, + ` // Maybe parameters to set the defaults?`, + ``, + ` dcl-ds person likeds(person_t);`, + ``, + ` // Set defaults`, + ` person.name = '';`, + ` person.age = 0;`, + ``, + ` return person;`, + `end-proc;`, + ``, + `dcl-proc PERSON_printNice;`, + ` dcl-pi *n;`, + ` person likeds(person_t);`, + ` end-pi;`, + ``, + ` printf(%trim(person.name) + ' ' + %char(person.age));`, + `end-proc;`, + ].join(`\n`); + + const parser = parserSetup(); + const cache = await parser.getDocs(uri, lines); + Linter.getErrors({ uri, content: lines }, { + CollectReferences: true, + }, cache); + + // Global checks + + const printf = cache.find(`printf`); + assert.strictEqual(printf.references.length, 1); + + const person_t = cache.find(`person_t`); + assert.strictEqual(person_t.references.length, 4); + + const myperson = cache.find(`myperson`); + assert.strictEqual(myperson.references.length, 4); + + const global_person = cache.find(`person`); + assert.strictEqual(global_person, null); + + // Proc A checks + + const PERSON_New = cache.find(`PERSON_New`); + assert.strictEqual(PERSON_New.references.length, 1); + const PERSON_New_person = PERSON_New.scope.find(`person`); + assert.strictEqual(PERSON_New_person.references.length, 3); + + // Proc B checks + + const PERSON_printNice = cache.find(`PERSON_printNice`); + assert.strictEqual(PERSON_printNice.references.length, 1); + const printNice_person = PERSON_printNice.scope.find(`person`); + assert.strictEqual(printNice_person.references.length, 2); + +} \ No newline at end of file From 26de9a665a5357e6745e2ee79ca3abef138ee53f Mon Sep 17 00:00:00 2001 From: Liam Barry Allan Date: Sat, 11 Feb 2023 11:24:01 -0500 Subject: [PATCH 2/4] Do not disturb existing storage of parameters Signed-off-by: Liam Barry Allan --- .vscode/launch.json | 3 +-- server/src/language/models/cache.js | 3 +-- tests/suite/linter.js | 1 - 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index fb91e299..d1082038 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -41,8 +41,7 @@ }, "env": { "INCLUDE_DIR": "${workspaceFolder}" - }, - "args": ["issue_204"] + } } ] } diff --git a/server/src/language/models/cache.js b/server/src/language/models/cache.js index a47f5349..60822e16 100644 --- a/server/src/language/models/cache.js +++ b/server/src/language/models/cache.js @@ -179,9 +179,8 @@ export default class Cache { fixProcedures() { if (this.procedures.length > 0) { this.procedures.forEach(proc => { - if (proc.scope) { + if (proc.scope && proc.subItems.length > 0) { proc.scope.parameters = [...proc.subItems]; - proc.subItems = []; proc.scope.fixProcedures(); } }) diff --git a/tests/suite/linter.js b/tests/suite/linter.js index f2db552a..09487a70 100644 --- a/tests/suite/linter.js +++ b/tests/suite/linter.js @@ -3463,5 +3463,4 @@ exports.issue_204 = async () => { assert.strictEqual(PERSON_printNice.references.length, 1); const printNice_person = PERSON_printNice.scope.find(`person`); assert.strictEqual(printNice_person.references.length, 2); - } \ No newline at end of file From 71ea536097a174f96f2499edcc0f2b91e51eab6b Mon Sep 17 00:00:00 2001 From: Liam Barry Allan Date: Sat, 11 Feb 2023 11:31:18 -0500 Subject: [PATCH 3/4] Additional check for parm subitems Signed-off-by: Liam Barry Allan --- tests/suite/linter.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/suite/linter.js b/tests/suite/linter.js index 09487a70..f0dec497 100644 --- a/tests/suite/linter.js +++ b/tests/suite/linter.js @@ -3456,6 +3456,7 @@ exports.issue_204 = async () => { assert.strictEqual(PERSON_New.references.length, 1); const PERSON_New_person = PERSON_New.scope.find(`person`); assert.strictEqual(PERSON_New_person.references.length, 3); + assert.strictEqual(PERSON_New_person.subItems.length, 2); // Proc B checks @@ -3463,4 +3464,5 @@ exports.issue_204 = async () => { assert.strictEqual(PERSON_printNice.references.length, 1); const printNice_person = PERSON_printNice.scope.find(`person`); assert.strictEqual(printNice_person.references.length, 2); + assert.strictEqual(printNice_person.subItems.length, 2); } \ No newline at end of file From f5d21b59bec911ed3ca938c7e317afee1090f24e Mon Sep 17 00:00:00 2001 From: Liam Barry Allan Date: Sat, 11 Feb 2023 11:52:52 -0500 Subject: [PATCH 4/4] Improved editing support for parameters and structs Signed-off-by: Liam Barry Allan --- server/src/providers/completionItem.ts | 49 ++++++++++++------------- server/src/providers/documentSymbols.ts | 6 +-- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/server/src/providers/completionItem.ts b/server/src/providers/completionItem.ts index f419d979..c3418bc7 100644 --- a/server/src/providers/completionItem.ts +++ b/server/src/providers/completionItem.ts @@ -28,7 +28,10 @@ export default async function completionItemProvider(handler: CompletionParams): const isFree = (document.getText(Range.create(0, 0, 0, 6)).toUpperCase() === `**FREE`); // If they're typing inside of a procedure, let's get the stuff from there too - const currentProcedure = doc.procedures.find(proc => lineNumber >= proc.range.start && lineNumber <= proc.range.end); + const currentProcedure = doc.procedures.find((proc, index) => + lineNumber >= proc.range.start && + (lineNumber <= proc.range.end+1 || index === doc.procedures.length-1) + ); const currentLine = document.getText(Range.create( handler.position.line, @@ -52,21 +55,19 @@ export default async function completionItemProvider(handler: CompletionParams): } } + // Ok, we have a 'preWord' (the name of the struct?) if (preWord) { let possibleStruct: Declaration | undefined; if (currentProcedure && currentProcedure.scope) { - const procScop = currentProcedure.scope; - - possibleStruct = currentProcedure.subItems.find(subitem => subitem.name.toUpperCase() === preWord && subitem.subItems.length > 0); - - if (!possibleStruct) { - possibleStruct = procScop.structs.find(struct => struct.name.toUpperCase() === preWord); - } - } - - if (!possibleStruct) { - possibleStruct = doc.structs.find(struct => struct.name.toUpperCase() === preWord); + const procScope = currentProcedure.scope; + + // Look at the parms or existing structs to find a possible reference + possibleStruct = [ + procScope.parameters.find(parm => parm.name.toUpperCase() === preWord && parm.subItems.length > 0), + procScope.structs.find(struct => struct.name.toUpperCase() === preWord), + doc.structs.find(struct => struct.name.toUpperCase() === preWord) + ].find(x => x); // find the first non-undefined item } if (possibleStruct && possibleStruct.keyword[`QUALIFIED`]) { @@ -101,6 +102,15 @@ export default async function completionItemProvider(handler: CompletionParams): } else { const expandScope = (localCache: Cache) => { + for (const subItem of localCache.parameters) { + const item = CompletionItem.create(`${subItem.name}`); + item.kind = CompletionItemKind.Variable; + item.insertText = subItem.name; + item.detail = [`parameter`, ...subItem.keywords].join(` `); + item.documentation = subItem.description; + items.push(item); + } + for (const procedure of localCache.procedures) { const item = CompletionItem.create(`${procedure.name}`); item.kind = CompletionItemKind.Function; @@ -189,19 +199,8 @@ export default async function completionItemProvider(handler: CompletionParams): expandScope(doc); - if (currentProcedure) { - for (const subItem of currentProcedure.subItems) { - const item = CompletionItem.create(`${subItem.name}`); - item.kind = CompletionItemKind.Variable; - item.insertText = subItem.name; - item.detail = [`parameter`, ...subItem.keywords].join(` `); - item.documentation = subItem.description; - items.push(item); - } - - if (currentProcedure.scope) { - expandScope(currentProcedure.scope); - } + if (currentProcedure && currentProcedure.scope) { + expandScope(currentProcedure.scope); } // Next, we're going to make some import suggestions for system APIs diff --git a/server/src/providers/documentSymbols.ts b/server/src/providers/documentSymbols.ts index 847e6e85..619131c9 100644 --- a/server/src/providers/documentSymbols.ts +++ b/server/src/providers/documentSymbols.ts @@ -28,7 +28,8 @@ export default async function documentSymbolProvider(handler: DocumentSymbolPara Range.create(proc.range.start, 0, proc.range.start, 0), ); - procDef.children = proc.subItems + if (proc.scope) { + procDef.children = proc.subItems .filter(subitem => subitem.position && subitem.position.path === currentPath) .map(subitem => DocumentSymbol.create( subitem.name, @@ -37,8 +38,7 @@ export default async function documentSymbolProvider(handler: DocumentSymbolPara Range.create(subitem.position.line, 0, subitem.position.line, 0), Range.create(subitem.position.line, 0, subitem.position.line, 0) )); - - if (proc.scope && procDef.children) { + procDef.children.push(...getScopeVars(proc.scope)); }