detect unused packages#2627
Conversation
unused packages are detected by checking whether a 'library' function call has a reads|calls edge pointing towards it
|
|
||
| export type UnusedImportMetadata = MergeableRecord; | ||
|
|
||
| const LibraryLoadFunctions = new Set(['library', 'require', 'requireNamespace', 'loadNamespace', 'attachNamespace', 'load_all', 'use', 'p_load']); |
There was a problem hiding this comment.
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 (
flowr/src/dataflow/environments/identifier.ts
Line 212 in 05e2ccc
| elements.getElements() | ||
| .filter(element => { | ||
| //if can't export we don't know if it used or not | ||
| const unknownIds = new Set<string>(); |
There was a problem hiding this comment.
please use the NodeId type to collect any kind of node ids! not all of them are strings!
| } | ||
| 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))) { |
There was a problem hiding this comment.
if we are detecting unused packages, why would we need this check?
| 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; |
There was a problem hiding this comment.
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
| name: 'Unused Import', | ||
| tags: [LintingRuleTag.Smell, LintingRuleTag.Readability], | ||
| certainty: LintingRuleCertainty.BestEffort, | ||
| description: 'Checks whether an imported package is used.', |
There was a problem hiding this comment.
I'd invert this to focus on it highlights packages which are not required for the code to run
| certainty: LintingRuleCertainty.BestEffort, | ||
| description: 'Checks whether an imported package is used.', | ||
| defaultConfig: { | ||
| pkgDb: undefined |
There was a problem hiding this comment.
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!
| 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; |
There was a problem hiding this comment.
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!
| 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) |
There was a problem hiding this comment.
why is there a commented out test?
There was a problem hiding this comment.
You can know use the Pkgdb builder for this!
| } | ||
| ]); console.log(result.linter.results["unused-import"]) | ||
| });*/ | ||
| assertLinter('a', parser, 'library(ggplot2)', 'unused-import', [ |
There was a problem hiding this comment.
thats the idea but please also add more complicated tests. e.g. one which uses some packages and does not use others!
|
Please also add documentation as described in https://github.com/flowr-analysis/flowr/wiki/create-linting-rules |
2 participants
unused packages are detected by checking whether a 'library' function call has a reads|calls edge pointing towards it