diff --git a/src/linter/linter-format.ts b/src/linter/linter-format.ts index 92eac796630..859acf7e01b 100644 --- a/src/linter/linter-format.ts +++ b/src/linter/linter-format.ts @@ -11,6 +11,7 @@ import type { SourceRange } from '../util/range'; import type { DataflowInformation } from '../dataflow/info'; import type { ControlFlowInformation } from '../control-flow/control-flow-graph'; import type { ReadonlyFlowrAnalysisProvider } from '../project/flowr-analyzer'; +import type { NodeId } from '../r-bridge/lang-4.x/ast/model/processing/node-id'; export interface LinterRuleInformation { /** Human-Readable name of the linting rule. */ @@ -94,11 +95,16 @@ export type LintQuickFix = LintQuickFixReplacement | LintQuickFixRemove; * A linting result for a single linting rule match. */ export interface LintingResult { - readonly certainty: LintingResultCertainty + /** The certainty of the linting result. */ + readonly certainty: LintingResultCertainty /** * If available, what to do to fix the linting result. */ - readonly quickFix?: LintQuickFix[] + readonly quickFix?: LintQuickFix[] + /** + * The node ID involved in this linting result, if applicable. + */ + readonly involvedId: NodeId | undefined } diff --git a/src/linter/rules/dataframe-access-validation.ts b/src/linter/rules/dataframe-access-validation.ts index ab71914743e..41cc8b06606 100644 --- a/src/linter/rules/dataframe-access-validation.ts +++ b/src/linter/rules/dataframe-access-validation.ts @@ -115,10 +115,11 @@ export const DATA_FRAME_ACCESS_VALIDATION = { })) .map(({ node, operand, ...accessed }) => ({ ...accessed, - access: node?.lexeme ?? '???', + involvedId: node?.info.id, + access: node?.lexeme ?? '???', ...(operand?.type === RType.Symbol ? { operand: operand.content } : {}), - range: node?.info.fullRange ?? node?.location ?? rangeFrom(-1, -1, -1, -1), - certainty: LintingResultCertainty.Certain + range: node?.info.fullRange ?? node?.location ?? rangeFrom(-1, -1, -1, -1), + certainty: LintingResultCertainty.Certain })); return { results, '.meta': metadata }; diff --git a/src/linter/rules/dead-code.ts b/src/linter/rules/dead-code.ts index 78757c9420c..505dd10c12f 100644 --- a/src/linter/rules/dead-code.ts +++ b/src/linter/rules/dead-code.ts @@ -44,7 +44,8 @@ export const DEAD_CODE = { .map(element => element.node.info.fullRange ?? element.node.location) .filter(isNotUndefined))) .map(range => ({ - certainty: LintingResultCertainty.Certain, + certainty: LintingResultCertainty.Certain, + involvedId: undefined, range })), '.meta': meta diff --git a/src/linter/rules/file-path-validity.ts b/src/linter/rules/file-path-validity.ts index 32c9bbfa0c1..0916b0e7d19 100644 --- a/src/linter/rules/file-path-validity.ts +++ b/src/linter/rules/file-path-validity.ts @@ -70,9 +70,10 @@ export const FILE_PATH_VALIDITY = { metadata.totalUnknown++; if(config.includeUnknown) { return [{ + involvedId: matchingRead.nodeId, range, - filePath: Unknown, - certainty: LintingResultCertainty.Uncertain + filePath: Unknown, + certainty: LintingResultCertainty.Uncertain }]; } else { return []; @@ -98,9 +99,10 @@ export const FILE_PATH_VALIDITY = { } return [{ + involvedId: matchingRead.nodeId, range, - filePath: matchingRead.value as string, - certainty: writesBefore && writesBefore.length && writesBefore.every(w => w === Ternary.Maybe) ? LintingResultCertainty.Uncertain : LintingResultCertainty.Certain + filePath: matchingRead.value as string, + certainty: writesBefore && writesBefore.length && writesBefore.every(w => w === Ternary.Maybe) ? LintingResultCertainty.Uncertain : LintingResultCertainty.Certain }]; }), '.meta': metadata diff --git a/src/linter/rules/function-finder-util.ts b/src/linter/rules/function-finder-util.ts index 07817d13c65..36daf85f4da 100644 --- a/src/linter/rules/function-finder-util.ts +++ b/src/linter/rules/function-finder-util.ts @@ -78,9 +78,10 @@ export const functionFinderUtil = { return { results: results.map(element => ({ - certainty: LintingResultCertainty.Certain, - function: element.target, - range: element.range + certainty: LintingResultCertainty.Certain, + involvedId: element.node.info.id, + function: element.target, + range: element.range })), '.meta': metadata }; diff --git a/src/linter/rules/naming-convention.ts b/src/linter/rules/naming-convention.ts index fc42439e85d..59329500add 100644 --- a/src/linter/rules/naming-convention.ts +++ b/src/linter/rules/naming-convention.ts @@ -212,7 +212,8 @@ export const NAMING_CONVENTION = { const fix = fixCasing(m.name, casing); return { ...m, - quickFix: fix ? createNamingConventionQuickFixes(data.dataflow.graph, id, fix, casing) : undefined + involvedId: id, + quickFix: fix ? createNamingConventionQuickFixes(data.dataflow.graph, id, fix, casing) : undefined }; }); return { diff --git a/src/linter/rules/seeded-randomness.ts b/src/linter/rules/seeded-randomness.ts index bf2c06ce46c..b1db4de42d6 100644 --- a/src/linter/rules/seeded-randomness.ts +++ b/src/linter/rules/seeded-randomness.ts @@ -84,6 +84,7 @@ export const SEEDED_RANDOMNESS = { .flatMap(element => enrichmentContent(element, Enrichment.CallTargets).targets.map(target => { metadata.consumerCalls++; return { + involvedId: element.node.info.id, range: element.node.info.fullRange as SourceRange, target: target as Identifier, searchElement: element @@ -147,9 +148,10 @@ export const SEEDED_RANDOMNESS = { metadata.callsWithOtherBranchProducers++; } return [{ - certainty: cdsOfProduces.size > 0 ? LintingResultCertainty.Uncertain : LintingResultCertainty.Certain, - function: element.target, - range: element.range + involvedId: element.involvedId, + certainty: cdsOfProduces.size > 0 ? LintingResultCertainty.Uncertain : LintingResultCertainty.Certain, + function: element.target, + range: element.range }]; }), '.meta': metadata diff --git a/src/linter/rules/unused-definition.ts b/src/linter/rules/unused-definition.ts index 530f9d8dedc..c34debc469c 100644 --- a/src/linter/rules/unused-definition.ts +++ b/src/linter/rules/unused-definition.ts @@ -122,10 +122,11 @@ export const UNUSED_DEFINITION = { // found an unused definition const variableName = element.node.lexeme; return [{ - certainty: LintingResultCertainty.Uncertain, + certainty: LintingResultCertainty.Uncertain, variableName, - range: element.node.info.fullRange ?? element.node.location ?? rangeFrom(-1, -1, -1, -1), - quickFix: buildQuickFix(element.node, data.dataflow.graph, data.normalize) + involvedId: element.node.info.id, + range: element.node.info.fullRange ?? element.node.location ?? rangeFrom(-1, -1, -1, -1), + quickFix: buildQuickFix(element.node, data.dataflow.graph, data.normalize) }] satisfies UnusedDefinitionResult[]; }).filter(isNotUndefined)), '.meta': metadata diff --git a/src/linter/rules/useless-loop.ts b/src/linter/rules/useless-loop.ts index c3d9231b53b..763761330b8 100644 --- a/src/linter/rules/useless-loop.ts +++ b/src/linter/rules/useless-loop.ts @@ -34,9 +34,10 @@ export const USELESS_LOOP = { }).filter(loop => onlyLoopsOnce(loop.node.info.id, dataflow.graph, cfg, normalize, analyzer.inspectContext()) ).map(res => ({ - certainty: LintingResultCertainty.Certain, - name: res.node.lexeme as string, - range: res.node.info.fullRange as SourceRange + certainty: LintingResultCertainty.Certain, + name: res.node.lexeme as string, + range: res.node.info.fullRange as SourceRange, + involvedId: res.node.info.id } satisfies UselessLoopResult)); return { diff --git a/src/queries/catalog/linter-query/linter-query-format.ts b/src/queries/catalog/linter-query/linter-query-format.ts index 280d0ed81ce..e8709dbef70 100644 --- a/src/queries/catalog/linter-query/linter-query-format.ts +++ b/src/queries/catalog/linter-query/linter-query-format.ts @@ -24,7 +24,7 @@ import type { FlowrConfigOptions } from '../../../config'; import type { ReplOutput } from '../../../cli/repl/commands/repl-main'; import type { CommandCompletions } from '../../../cli/repl/core'; import { fileProtocol } from '../../../r-bridge/retriever'; -import { getGuardIssueUrl } from '../../../util/assert'; +import { getGuardIssueUrl, isNotUndefined } from '../../../util/assert'; export interface LinterQuery extends BaseQueryFormat { readonly type: 'linter'; @@ -141,7 +141,15 @@ export const LinterQueryDefinition = { }) ).description('The rules to lint for. If unset, all rules will be included.') }).description('The linter query lints for the given set of rules and returns the result.'), - flattenInvolvedNodes: () => [] + flattenInvolvedNodes: (queryResults) => { + const out = queryResults as LinterQueryResult; + return Object.values(out.results).flatMap(v => { + if(isLintingResultsError(v)) { + return []; + } + return v.results.map(v => v.involvedId); + }).filter(isNotUndefined); + } } as const satisfies SupportedQuery<'linter'>; function addLintingRuleResult(ruleName: Name, results: LintingResults, result: string[]) { diff --git a/test/functionality/_helper/linter.ts b/test/functionality/_helper/linter.ts index c390ddd6398..de16245ac02 100644 --- a/test/functionality/_helper/linter.ts +++ b/test/functionality/_helper/linter.ts @@ -20,6 +20,16 @@ import { FlowrAnalyzerBuilder } from '../../../src/project/flowr-analyzer-builde import type { FlowrFileProvider } from '../../../src/project/context/flowr-file'; import { FlowrInlineTextFile } from '../../../src/project/context/flowr-file'; +function cleanUpLintingResult( + result: LintingRuleResult | Omit, 'involvedId'> +): Omit, 'involvedId'> { + if('involvedId' in result) { + const { involvedId: _drop, ...rest } = result; + return rest; + } + return result; +} + /** * @@ -29,7 +39,7 @@ export function assertLinter( parser: KnownParser, code: string, ruleName: Name, - expected: LintingRuleResult[] | ((df: DataflowInformation, ast: NormalizedAst) => LintingRuleResult[]), + expected: Omit, 'involvedId'>[] | ((df: DataflowInformation, ast: NormalizedAst) => Omit, 'involvedId'>[]), expectedMetadata?: LintingRuleMetadata, lintingRuleConfig?: DeepPartial> & { useAsFilePath?: string, addFiles?: FlowrFileProvider[] } ) { @@ -76,7 +86,7 @@ export function assertLinter( } try { - assert.deepEqual(results.results, expected, `Expected ${ruleName} to return ${JSON.stringify(expected)}, but got ${JSON.stringify(results)}`); + assert.deepEqual(results.results.map(cleanUpLintingResult), expected.map(cleanUpLintingResult), `Expected ${ruleName} to return ${JSON.stringify(expected)}, but got ${JSON.stringify(results)}`); } catch(e) { console.error('dfg:', graphToMermaidUrl((await analyzer.dataflow()).graph)); throw e; diff --git a/test/functionality/linter/lint-dataframe-access-validation.test.ts b/test/functionality/linter/lint-dataframe-access-validation.test.ts index 32087d76aa6..2227c3480aa 100644 --- a/test/functionality/linter/lint-dataframe-access-validation.test.ts +++ b/test/functionality/linter/lint-dataframe-access-validation.test.ts @@ -93,7 +93,7 @@ print(df3${access}) } }); describe('Invalid data frame access', () => { - type ExpectedLinterResult = Omit; + type ExpectedLinterResult = Omit; const testCases: [string, ExpectedLinterResult][] = [ ['df <- data.frame(id = 1:5, name = "A")\ndf$score', { type: 'column', accessed: 'score', access: '$', operand: 'df', range: rangeFrom(2, 1, 2, 8) }],