Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 3 additions & 1 deletion src/linter/linter-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { SOFTWARE_HAS_LICENSE } from './rules/software-has-license';
import { SOFTWARE_HAS_TESTS } from './rules/software-has-tests';
import { NO_LEAKED_CREDENTIALS } from './rules/no-leaked-credentials';
import { UNDEFINED_SYMBOL } from './rules/undefined-symbol';
import { UNUSED_IMPORT } from './rules/unused-import';

/**
* The registry of currently supported linting rules.
Expand All @@ -38,7 +39,8 @@ export const LintingRules = {
'software-has-license': SOFTWARE_HAS_LICENSE,
'software-has-tests': SOFTWARE_HAS_TESTS,
'no-leaked-credentials': NO_LEAKED_CREDENTIALS,
'undefined-symbol': UNDEFINED_SYMBOL
'undefined-symbol': UNDEFINED_SYMBOL,
'unused-import': UNUSED_IMPORT
} as const;

export type LintingRuleNames = keyof typeof LintingRules;
Expand Down
89 changes: 89 additions & 0 deletions src/linter/rules/unused-import.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import {
LintingPrettyPrintContext,
type LintingResult,
LintingResultCertainty,
type LintingRule,
LintingRuleCertainty
} from '../linter-format';
import { SourceLocation } from '../../util/range';
import type { MergeableRecord } from '../../util/objects';
import { Q } from '../../search/flowr-search-builder';
import { LintingRuleTag } from '../linter-tags';
import { isNotUndefined, isUndefined } from '../../util/assert';
import type { Writable } from 'ts-essentials';
import { VertexType } from '../../dataflow/graph/vertex';
import { getOriginInDfg, OriginType } from '../../dataflow/origin/dfg-get-origin';
import { EdgeType } from '../../dataflow/graph/edge';
import { Identifier } from '../../dataflow/environments/identifier';
import type { PkgDbSource } from '../../project/plugins/package-version-plugins/flowr-analyzer-package-versions-pkgdb-plugin';

export type UnusedImportResult = LintingResult;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The output should also report which version of the package was used (from the package db) to mark it as unused because maybe it was useful in an older version etc. Also it

  • should be clarified in the doc that you need a package db for this to warn
  • only packages for which we have a package db entry (i.e., which are not marked as unknown) should be considered!


export interface UnusedImportConfig extends MergeableRecord {
pkgDb: PkgDbSource | undefined
};

export type UnusedImportMetadata = MergeableRecord;

const LibraryLoadFunctions = new Set(['library', 'require', 'requireNamespace', 'loadNamespace', 'attachNamespace', 'load_all', 'use', 'p_load']);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please do not harcode them here but use https://github.com/flowr-analysis/flowr/blob/main/src/queries/catalog/dependencies-query/function-info/library-functions.ts (the LibraryFunctions object) to match them package sensitive => also use Identifier.toQualified if possible please (

toQualified(this: void, origins: readonly Origin[] | undefined): Identifier | undefined {
)




export const UNUSED_IMPORT = {
createSearch: () => Q.var('library')
Comment thread
lou7202 marked this conversation as resolved.
Outdated
.filter(VertexType.FunctionCall),
processSearchResult: async(elements, _config, data) => {
const dataflow = await data.dataflow();
return {
results:
elements.getElements()
.filter(element => {
//if can't export we don't know if it used or not
const unknownIds = new Set<string>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use the NodeId type to collect any kind of node ids! not all of them are strings!

for(const e of dataflow.graph.unknownSideEffects) {
unknownIds.add(String(typeof e === 'object' ? e.id : e));
}
if(unknownIds.size > 0) {
for(const [id, v] of dataflow.graph.verticesOfType(VertexType.FunctionCall)) {
if(LibraryLoadFunctions.has(Identifier.getName(v.name)) && unknownIds.has(String(id))) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are detecting unused packages, why would we need this check?

return false;
}
}
}

const origins = getOriginInDfg(dataflow.graph, element.node.info.id);
if(isNotUndefined(origins)) {
const builtIn = origins.every(e => e.type === OriginType.BuiltInFunctionOrigin);
if(!builtIn){
return false;
}
}
return isUndefined(dataflow.graph.ingoingEdges(element.node.info.id)) || dataflow.graph.ingoingEdges(element.node.info.id)?.values().filter(edge => edge.types === EdgeType.Calls + EdgeType.Reads).toArray().length === 0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can take the ingoing edges but you can also iterate over all library calls identified by fromQuery and check whether they have an edge pointing to them.
Also id' argue that includesType of a read edge should suffice

})/*.map(v => {
console.log(v)
console.log('loc', SourceLocation.fromNode(v.node))
return v
})*/
.map(element => ({
certainty: LintingResultCertainty.Uncertain,
involvedId: element.node.info.id,
loc: SourceLocation.fromNode(element.node)
}))
.filter(element => isNotUndefined(element.loc)) as Writable<UnusedImportResult>[],
'.meta': {}
};
},
prettyPrint: {
[LintingPrettyPrintContext.Query]: result => `Code at ${SourceLocation.format(result.loc)}`,
[LintingPrettyPrintContext.Full]: result => `Code at ${SourceLocation.format(result.loc)} is an unused package.`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this reads... weird? why not just "Import at is unused"

},
info: {
name: 'Unused Import',
tags: [LintingRuleTag.Smell, LintingRuleTag.Readability],
certainty: LintingRuleCertainty.BestEffort,
description: 'Checks whether an imported package is used.',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd invert this to focus on it highlights packages which are not required for the code to run

defaultConfig: {
pkgDb: undefined

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is used nowhere? and please remove it here. flowR has a config for the pkgDB and you can control the used pkg db with the flowR config (https://github.com/flowr-analysis/flowr/wiki/package-database)! what we do need however, is the ability to whitelist imports (e.g. packages that do something on load that could be needed) which should not be marked as unused!

}
}
} as const satisfies LintingRule<UnusedImportResult, UnusedImportMetadata, UnusedImportConfig>;
74 changes: 74 additions & 0 deletions test/functionality/linter/lint-unused-import.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { describe } from 'vitest';
import { assertLinter } from '../_helper/linter';
import { withTreeSitter } from '../_helper/shell';
import { PkgDatabase } from '../../../src/project/plugins/package-version-plugins/pkgdb';
import { LintingResultCertainty } from '../../../src/linter/linter-format';

//const ggplot2Callable = ['+', 'ggplot', 'aes', 'geom_point', 'geom_line', 'theme_bw', 'coord_cartesian', 'ggsave', 'fortify', 'scale_type'];
/*const namespaceInfo = setCallable(FlowrNamespaceFile.from(new FlowrInlineTextFile('NAMESPACE', `S3method(fortify,data.frame)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is there a commented out test?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can know use the Pkgdb builder for this!

S3method(fortify,lm)
S3method(fortify,default)
S3method(fortify,map)
S3method(scale_type,Date)
S3method(scale_type,POSIXt)
S3method(scale_type,character)
S3method(scale_type,numeric)
S3method(scale_type,factor)
S3method(scale_type,default)
S3method(ggplot,default)
S3method(print,ggproto)
S3method(print,rel)
export("+")
export(ggplot)
export(aes)
export(geom_point)
export(geom_line)
export(theme_bw)
export(coord_cartesian)
export(ggsave)
export(fortify)
export(scale_type)
exportPattern("^[^\\\\.]\\\\.*$")
import(grid)
import(rlang)
importFrom(scales,alpha)
importFrom(stats,setNames)`)).content().current, ggplot2Callable);*/

const pkgDatabase = PkgDatabase.fromObject({
format: 'flowr-pkgdb',
schema: 4,
scope: 'latest',
content: { version: 1, date: '2026-05-23', hash: 'x', generated: 0, packages: 1, versions: 1 },
strings: [],
pkgs: { ggplot2: ['3.5.1', ['ggplot', 'aes', 'geom_point']], random1: ['1.0.0', ['test1', 'test2']] }
});

describe('flowR linter', withTreeSitter(parser => {
describe('unused import', () => {
/*test('Several libraries, check env structure', async() => {
const analyzer = await new FlowrAnalyzerBuilder()
.setParser(parser)
.build();
analyzer.context().deps.addDependency(new Package({
name: 'ggplot2',
namespaceInfo: namespaceInfo
}));
analyzer.context().deps.addDependency(new Package({
name: 'random1',
namespaceInfo: setCallable(FlowrNamespaceFile.from(new FlowrInlineTextFile('NAMESPACE', 'export(test1)\nexport(test2)')).content().current, ['test1', 'test2'])
}));
analyzer.addRequest('library(ggplot2)\nlibrary(random1)\nggplot()');
const df = await analyzer.dataflow();
const result = await analyzer.query([{
type: 'linter'
}
]); console.log(result.linter.results["unused-import"])
});*/
assertLinter('a', parser, 'library(ggplot2)', 'unused-import', [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats the idea but please also add more complicated tests. e.g. one which uses some packages and does not use others!

{
certainty: LintingResultCertainty.Uncertain,
loc: [1, 1, 1, 16],
},
], undefined, { pkgDb: pkgDatabase });
});
}));
Loading