Skip to content

Commit

Permalink
Merge pull request #331 from codefori/fix/string_dupe_variable
Browse files Browse the repository at this point in the history
Fix for string constant using wrong variable
  • Loading branch information
worksofliam authored Sep 21, 2024
2 parents 2d5e089 + dbaaae3 commit 86e532b
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 20 deletions.
25 changes: 5 additions & 20 deletions language/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export default class Linter {

const selectBlocks: SelectBlock[] = [];

const stringLiterals: { value: string, definition?: string, list: { offset: Offset }[] }[] = [];
const stringLiterals: { value: string, list: { line: number, offset: Offset }[] }[] = [];

let directiveScope = 0;
let currentRule = skipRules.none;
Expand Down Expand Up @@ -414,23 +414,6 @@ export default class Linter {
});
}
}

if (rules.StringLiteralDupe) {
if (statement[2].type === `string`) {
let foundBefore = stringLiterals.find(literal => literal.value === statement[2].value);

// If it does not exist on our list, we can add it
if (!foundBefore) {
foundBefore = {
definition: value,
value: statement[2].value,
list: []
};

stringLiterals.push(foundBefore);
}
}
}
break;

case `DCL-PI`:
Expand Down Expand Up @@ -989,6 +972,7 @@ export default class Linter {

// Then add our new found literal location to the list
foundBefore.list.push({
line: lineNumber,
offset: { position: part.range.start, end: part.range.end }
});
}
Expand Down Expand Up @@ -1080,10 +1064,11 @@ export default class Linter {
stringLiterals.forEach(literal => {
if (literal.list.length >= literalMinimum) {
literal.list.forEach(location => {
const possibleConst = globalScope.findConstByValue(location.line, literal.value);
errors.push({
...location,
offset: location.offset,
type: `StringLiteralDupe`,
newValue: literal.definition
newValue: possibleConst ? possibleConst.name : undefined
});
});
}
Expand Down
22 changes: 22 additions & 0 deletions language/models/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,28 @@ export default class Cache {
}
}

findConstByValue(lineNumber: number, value: string) {
const upperValue = value.toUpperCase(); // Keywords are stored in uppercase

// If they're typing inside of a procedure, let's get the stuff from there too
const currentProcedure = this.procedures.find(proc => lineNumber >= proc.range.start && lineNumber <= proc.range.end);

if (currentProcedure && currentProcedure.scope) {
console.log(currentProcedure.scope.constants);
const localDef = currentProcedure.scope.constants.find(def => def.keyword[upperValue] === true);

if (localDef) {
return localDef;
}
}

const globalDef = this.constants.find(def => def.keyword[upperValue] === true);

if (globalDef) {
return globalDef;
}
}

/**
* Move all procedure subItems (the paramaters) into the cache
*/
Expand Down
58 changes: 58 additions & 0 deletions tests/suite/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3345,4 +3345,62 @@ test('linter with non-free copybook', async () => {
}, cache);

expect(errors.length).toBe(0);
});

test('constant replace picking up wrong variable #330', async () => {
const lines = [
`**Free`,
`Ctl-Opt Main(Proc_Name);`,
`Ctl-Opt Debug Option(*SrcStmt:*NoDebugIO);`,
`Ctl-Opt ActGrp(*Caller);`,
`Ctl-opt ExtBinInt(*Yes); `,
``,
`Dcl-Proc Proc_1; `,
``,
` Dcl-Pi Proc_1 Char(20);`,
``,
` End-Pi;`,
` `,
` Dcl-s altError Char(20);`,
``,
` Dcl-c basicError 'Invalid credentials';`,
``,
` altError = 'Invalid credentials';`,
``,
` Return basicError;`,
``,
`On-Exit;`,
`End-Proc Proc_1;`,
``,
`Dcl-Proc Proc_2;`,
``,
` Dcl-Pi Proc_2 Char(20);`,
``,
` End-Pi;`,
``,
` Return 'Invalid credentials';`,
``,
`On-Exit;`,
`End-Proc Proc_2;`,
].join(`\n`);

const cache = await parser.getDocs(uri, lines, { ignoreCache: true, withIncludes: true });
const { errors } = Linter.getErrors({ uri, content: lines }, {
StringLiteralDupe: true
}, cache);

console.log(errors);
expect(errors.length).toBe(2);

expect(errors[0]).toMatchObject({
offset: { position: 270, end: 291 },
type: 'StringLiteralDupe',
newValue: 'basicError'
});

expect(errors[1]).toMatchObject({
offset: { position: 408, end: 429 },
type: 'StringLiteralDupe',
newValue: undefined
});
})

0 comments on commit 86e532b

Please sign in to comment.