Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/linter/linter-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Config extends MergeableRecord = never> {
/** Human-Readable name of the linting rule. */
Expand Down Expand Up @@ -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
}


Expand Down
7 changes: 4 additions & 3 deletions src/linter/rules/dataframe-access-validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
3 changes: 2 additions & 1 deletion src/linter/rules/dead-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions src/linter/rules/file-path-validity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 [];
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions src/linter/rules/function-finder-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down
3 changes: 2 additions & 1 deletion src/linter/rules/naming-convention.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 5 additions & 3 deletions src/linter/rules/seeded-randomness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions src/linter/rules/unused-definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions src/linter/rules/useless-loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 10 additions & 2 deletions src/queries/catalog/linter-query/linter-query-format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<Name extends LintingRuleNames>(ruleName: Name, results: LintingResults<Name>, result: string[]) {
Expand Down
14 changes: 12 additions & 2 deletions test/functionality/_helper/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Name extends LintingRuleNames>(
result: LintingRuleResult<Name> | Omit<LintingRuleResult<Name>, 'involvedId'>
): Omit<LintingRuleResult<Name>, 'involvedId'> {
if('involvedId' in result) {
const { involvedId: _drop, ...rest } = result;
return rest;
}
return result;
}


/**
*
Expand All @@ -29,7 +39,7 @@ export function assertLinter<Name extends LintingRuleNames>(
parser: KnownParser,
code: string,
ruleName: Name,
expected: LintingRuleResult<Name>[] | ((df: DataflowInformation, ast: NormalizedAst) => LintingRuleResult<Name>[]),
expected: Omit<LintingRuleResult<Name>, 'involvedId'>[] | ((df: DataflowInformation, ast: NormalizedAst) => Omit<LintingRuleResult<Name>, 'involvedId'>[]),
expectedMetadata?: LintingRuleMetadata<Name>,
lintingRuleConfig?: DeepPartial<LintingRuleConfig<Name>> & { useAsFilePath?: string, addFiles?: FlowrFileProvider[] }
) {
Expand Down Expand Up @@ -76,7 +86,7 @@ export function assertLinter<Name extends LintingRuleNames>(
}

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ print(df3${access})
}
});
describe('Invalid data frame access', () => {
type ExpectedLinterResult = Omit<DataFrameAccessValidationResult, 'certainty' | 'quickFix'>;
type ExpectedLinterResult = Omit<DataFrameAccessValidationResult, 'certainty' | 'quickFix' | 'involvedId'>;

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) }],
Expand Down
Loading