From 12d1bbbc9f52f9e40f9710c1fd88d8b0f1ee26bb Mon Sep 17 00:00:00 2001 From: worksofliam Date: Sat, 20 Jan 2024 11:40:05 -0500 Subject: [PATCH 1/5] Initial work on extract procedure --- .../src/providers/linter/codeActions.ts | 9 ++- .../server/src/providers/linter/index.ts | 77 +++++++++++++++++++ language/models/cache.ts | 31 ++++++-- package-lock.json | 4 +- 4 files changed, 111 insertions(+), 10 deletions(-) diff --git a/extension/server/src/providers/linter/codeActions.ts b/extension/server/src/providers/linter/codeActions.ts index 0001c91c..50c4aa29 100644 --- a/extension/server/src/providers/linter/codeActions.ts +++ b/extension/server/src/providers/linter/codeActions.ts @@ -1,5 +1,5 @@ -import { CodeAction, CodeActionParams, Range } from 'vscode-languageserver'; -import { getActions, refreshLinterDiagnostics } from '.'; +import { CodeAction, CodeActionKind, CodeActionParams, Range } from 'vscode-languageserver'; +import { getActions, getExtractProcedureAction, refreshLinterDiagnostics } from '.'; import { documents, parser } from '..'; export default async function codeActionsProvider(params: CodeActionParams): Promise { @@ -13,6 +13,11 @@ export default async function codeActionsProvider(params: CodeActionParams): Pro const docs = await parser.getDocs(document.uri); if (docs) { + const extractOption = getExtractProcedureAction(document, docs, range); + if (extractOption) { + return [extractOption]; + } + const detail = await refreshLinterDiagnostics(document, docs, false); if (detail) { const fixErrors = detail.errors.filter(error => diff --git a/extension/server/src/providers/linter/index.ts b/extension/server/src/providers/linter/index.ts index a7c1fcd8..5100256b 100644 --- a/extension/server/src/providers/linter/index.ts +++ b/extension/server/src/providers/linter/index.ts @@ -431,4 +431,81 @@ export function getActions(document: TextDocument, errors: IssueRange[]) { }); return actions; +} + +export function getExtractProcedureAction(document: TextDocument, docs: Cache, range: Range): CodeAction|undefined { + if (range.end.line > range.start.line) { + const references = docs.referencesInRange({position: document.offsetAt(range.start), end: document.offsetAt(range.end)}); + const validRefs = references.filter(ref => [`struct`, `subitem`, `variable`].includes(ref.dec.type)); + + if (validRefs.length > 0) { + const linesRange = Range.create(range.start.line, 0, range.end.line, 1000); + const lastLine = document.offsetAt({line: document.lineCount, character: 0}); + + const nameDiffSize = 1; // Always once since we only add 'p' at the start + const newParamNames = validRefs.map(ref => `p${ref.dec.name}`); + let procedureBody = document.getText(range); + + // Fix the found offset lengths to be relative to the new procedure + for (let i = validRefs.length - 1; i >= 0; i--) { + for (let y = validRefs[i].refs.length - 1; y >= 0; y--) { + validRefs[i].refs[y] = { + position: validRefs[i].refs[y].position - document.offsetAt(range.start), + end: validRefs[i].refs[y].end - document.offsetAt(range.start) + }; + } + } + + // Then let's fix the references to use the new names + for (let i = validRefs.length - 1; i >= 0; i--) { + for (let y = validRefs[i].refs.length - 1; y >= 0; y--) { + const ref = validRefs[i].refs[y]; + + procedureBody = procedureBody.slice(0, ref.position) + newParamNames[i] + procedureBody.slice(ref.end); + + // Then we need to update the offset of the next references + for (let z = i + 1; z < validRefs.length; z++) { + for (let x = validRefs[z].refs.length - 1; x >= 0; x--) { + if (validRefs[z].refs[x].position > ref.position) { + validRefs[z].refs[x] = { + position: validRefs[z].refs[x].position + (newParamNames[i].length - nameDiffSize), + end: validRefs[z].refs[x].end + (newParamNames[i].length - nameDiffSize) + }; + } + } + } + } + } + + const newProcedure = [ + `Dcl-Proc NewProcedure;`, + ` Dcl-Pi *N;`, + ...validRefs.map((ref, i) => ` ${newParamNames[i]} ${ref.dec.type === `struct` ? `LikeDS` : `Like`}(${ref.dec.name});`), + ` End-Pi;`, + ``, + procedureBody, + `End-Proc;` + ].join(`\n`) + + const newAction = CodeAction.create(`Extract to new procedure`, CodeActionKind.RefactorExtract); + + // First do the exit + newAction.edit = { + changes: { + [document.uri]: [ + TextEdit.replace(linesRange, `NewProcedure(${validRefs.map(r => r.dec.name).join(`:`)});`), + TextEdit.insert(document.positionAt(lastLine), `\n\n`+newProcedure) + ] + }, + }; + + // Then format the document + newAction.command = { + command: `editor.action.formatDocument`, + title: `Format` + }; + + return newAction; + } + } } \ No newline at end of file diff --git a/language/models/cache.ts b/language/models/cache.ts index d2f723e7..1d013d44 100644 --- a/language/models/cache.ts +++ b/language/models/cache.ts @@ -1,5 +1,5 @@ import { indicators1 } from "../../tests/suite"; -import { CacheProps, IncludeStatement, Keywords } from "../parserTypes"; +import { CacheProps, IncludeStatement, Keywords, Offset } from "../parserTypes"; import Declaration from "./declaration"; const newInds = () => { @@ -199,18 +199,37 @@ export default class Cache { } } - static referenceByOffset(scope: Cache, offset: number): Declaration|undefined { + referencesInRange(range: Offset): { dec: Declaration, refs: Offset[] }[] { + let list: { dec: Declaration, refs: Offset[] }[] = []; + + for (let i = range.position; i <= range.end; i++) { + const ref = Cache.referenceByOffset(this, i); + if (ref) { + // No duplicates allowed + if (list.some(item => item.dec.name === ref.name)) continue; + + list.push({ + dec: ref, + refs: ref.references.filter(r => r.offset.position >= range.position && r.offset.end <= range.end).map(r => r.offset) + }) + }; + } + + return list; + } + + static referenceByOffset(scope: Cache, offset: number): Declaration | undefined { const props: (keyof Cache)[] = [`parameters`, `subroutines`, `procedures`, `files`, `variables`, `structs`, `constants`, `indicators`]; - + for (const prop of props) { const list = scope[prop] as unknown as Declaration[]; for (const def of list) { let possibleRef: boolean; - + // Search top level possibleRef = def.references.some(r => offset >= r.offset.position && offset <= r.offset.end); if (possibleRef) return def; - + // Search any subitems if (def.subItems.length > 0) { for (const subItem of def.subItems) { @@ -218,7 +237,7 @@ export default class Cache { if (possibleRef) return subItem; } } - + // Search scope if any if (def.scope) { const inScope = Cache.referenceByOffset(def.scope, offset); diff --git a/package-lock.json b/package-lock.json index 2e2c6570..e4a68173 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "vscode-rpgle", - "version": "0.20.2", + "version": "0.24.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "vscode-rpgle", - "version": "0.20.2", + "version": "0.24.0", "hasInstallScript": true, "license": "MIT", "devDependencies": { From f8522c11e93bc3c6868d3ee24e97b1e91aea585a Mon Sep 17 00:00:00 2001 From: worksofliam Date: Sat, 20 Jan 2024 13:43:19 -0500 Subject: [PATCH 2/5] Correct broken logic for replacing variables --- extension/server/src/providers/linter/index.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/extension/server/src/providers/linter/index.ts b/extension/server/src/providers/linter/index.ts index 5100256b..365a481a 100644 --- a/extension/server/src/providers/linter/index.ts +++ b/extension/server/src/providers/linter/index.ts @@ -462,14 +462,15 @@ export function getExtractProcedureAction(document: TextDocument, docs: Cache, r const ref = validRefs[i].refs[y]; procedureBody = procedureBody.slice(0, ref.position) + newParamNames[i] + procedureBody.slice(ref.end); + ref.end += nameDiffSize; // Then we need to update the offset of the next references - for (let z = i + 1; z < validRefs.length; z++) { + for (let z = i - 1; z >= 0; z--) { for (let x = validRefs[z].refs.length - 1; x >= 0; x--) { - if (validRefs[z].refs[x].position > ref.position) { + if (validRefs[z].refs[x].position > ref.end) { validRefs[z].refs[x] = { - position: validRefs[z].refs[x].position + (newParamNames[i].length - nameDiffSize), - end: validRefs[z].refs[x].end + (newParamNames[i].length - nameDiffSize) + position: validRefs[z].refs[x].position + nameDiffSize, + end: validRefs[z].refs[x].end + nameDiffSize }; } } From ce1020b613e060fb20b720f0e203bc9cdfce6f26 Mon Sep 17 00:00:00 2001 From: worksofliam Date: Sun, 21 Jan 2024 10:19:39 -0500 Subject: [PATCH 3/5] Use line offset for all ranges --- extension/server/src/providers/linter/index.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/extension/server/src/providers/linter/index.ts b/extension/server/src/providers/linter/index.ts index 365a481a..38313231 100644 --- a/extension/server/src/providers/linter/index.ts +++ b/extension/server/src/providers/linter/index.ts @@ -444,14 +444,16 @@ export function getExtractProcedureAction(document: TextDocument, docs: Cache, r const nameDiffSize = 1; // Always once since we only add 'p' at the start const newParamNames = validRefs.map(ref => `p${ref.dec.name}`); - let procedureBody = document.getText(range); + let procedureBody = document.getText(linesRange); + + const rangeStartOffset = document.offsetAt(linesRange.start); // Fix the found offset lengths to be relative to the new procedure for (let i = validRefs.length - 1; i >= 0; i--) { for (let y = validRefs[i].refs.length - 1; y >= 0; y--) { validRefs[i].refs[y] = { - position: validRefs[i].refs[y].position - document.offsetAt(range.start), - end: validRefs[i].refs[y].end - document.offsetAt(range.start) + position: validRefs[i].refs[y].position - rangeStartOffset, + end: validRefs[i].refs[y].end - rangeStartOffset }; } } From 18278d36990d39b2f426b5f5d712edf0c38b02f7 Mon Sep 17 00:00:00 2001 From: worksofliam Date: Sun, 21 Jan 2024 10:35:14 -0500 Subject: [PATCH 4/5] New range test --- tests/suite/linter.js | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/suite/linter.js b/tests/suite/linter.js index 4add3ec3..894b3b66 100644 --- a/tests/suite/linter.js +++ b/tests/suite/linter.js @@ -3463,4 +3463,41 @@ exports.on_excp_2 = async () => { expectedIndent: 2, currentIndent: 0 }); +} + +exports.range_1 = async () => { + const lines = [ + `**free`, + `ctl-opt debug option(*nodebugio: *srcstmt) dftactgrp(*no) actgrp(*caller)`, + `main(Main);`, + `dcl-s x timestamp;`, + `dcl-s y timestamp;`, + `dcl-proc Main;`, + ` dsply %CHAR(CalcDiscount(10000));`, + ` dsply %char(CalcDiscount(1000));`, + ` x = %TIMESTAMP(y);`, + ` y = %TimeStamp(x);`, + ` return;`, + `end-proc;`, + ].join(`\n`); + + const cache = await parser.getDocs(uri, lines, {ignoreCache: true, withIncludes: true}); + Linter.getErrors({ uri, content: lines }, { + CollectReferences: true + }, cache); + + const rangeRefs = cache.referencesInRange({position: 220, end: 260}); + assert.strictEqual(rangeRefs.length, 2); + assert.ok(rangeRefs[0].dec.name === `x`); + assert.ok(rangeRefs[1].dec.name === `y`); + + assert.deepStrictEqual(rangeRefs[0].refs, [ + { position: 220, end: 221 }, + { position: 256, end: 257 } + ]); + + assert.deepStrictEqual(rangeRefs[1].refs, [ + { position: 235, end: 236 }, + { position: 241, end: 242 } + ]); } \ No newline at end of file From e4cc1d1002df97ca380f114d6228039b19c815e5 Mon Sep 17 00:00:00 2001 From: worksofliam Date: Mon, 22 Jan 2024 10:34:02 -0500 Subject: [PATCH 5/5] Correct range to get references --- extension/server/src/providers/linter/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extension/server/src/providers/linter/index.ts b/extension/server/src/providers/linter/index.ts index 38313231..ed4018e3 100644 --- a/extension/server/src/providers/linter/index.ts +++ b/extension/server/src/providers/linter/index.ts @@ -435,11 +435,11 @@ export function getActions(document: TextDocument, errors: IssueRange[]) { export function getExtractProcedureAction(document: TextDocument, docs: Cache, range: Range): CodeAction|undefined { if (range.end.line > range.start.line) { - const references = docs.referencesInRange({position: document.offsetAt(range.start), end: document.offsetAt(range.end)}); + const linesRange = Range.create(range.start.line, 0, range.end.line, 1000); + const references = docs.referencesInRange({position: document.offsetAt(linesRange.start), end: document.offsetAt(linesRange.end)}); const validRefs = references.filter(ref => [`struct`, `subitem`, `variable`].includes(ref.dec.type)); if (validRefs.length > 0) { - const linesRange = Range.create(range.start.line, 0, range.end.line, 1000); const lastLine = document.offsetAt({line: document.lineCount, character: 0}); const nameDiffSize = 1; // Always once since we only add 'p' at the start