-
Notifications
You must be signed in to change notification settings - Fork 10
Add NAMESPACE-file-plugin #1983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
01a2d88
cd40cbd
2544701
d62c6c3
3717b1e
d8a96bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import { FlowrAnalyzerFilePlugin } from './flowr-analyzer-file-plugin'; | ||
| import { SemVer } from 'semver'; | ||
| import type { PathLike } from 'fs'; | ||
| import type { FlowrAnalyzerContext } from '../../context/flowr-analyzer-context'; | ||
| import type { FlowrFileProvider } from '../../context/flowr-file'; | ||
| import { SpecialFileRole } from '../../context/flowr-file'; | ||
| import { FlowrNamespaceFile } from './flowr-namespace-file'; | ||
|
|
||
| /** | ||
| * This plugin provides support for R `NAMESPACE` files. | ||
| */ | ||
| export class FlowrAnalyzerNamespaceFilePlugin extends FlowrAnalyzerFilePlugin { | ||
| public readonly name = 'flowr-analyzer-namespace-file-plugin'; | ||
| public readonly description = 'This plugin provides support for NAMESPACE files and extracts their content into the NAMESPACEFormat.'; | ||
| public readonly version = new SemVer('0.1.0'); | ||
|
|
||
| public applies(file: PathLike): boolean { | ||
| return /^(NAMESPACE|NAMESPACE\.txt)$/i.test(file.toString().split(/[/\\]/).pop() ?? ''); | ||
| } | ||
|
|
||
| public process(_ctx: FlowrAnalyzerContext, file: FlowrFileProvider<string>): FlowrNamespaceFile { | ||
| const f = FlowrNamespaceFile.from(file, SpecialFileRole.Namespace); | ||
| // already load it here | ||
| f.content(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think we do not want this here :P this should be also removed from the description wrapper i guess 🤣 |
||
| return f; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,50 @@ | ||||||
| import type { FlowrFileProvider, SpecialFileRole } from '../../context/flowr-file'; | ||||||
| import { FlowrFile } from '../../context/flowr-file'; | ||||||
| import { parseNAMESPACE } from '../../../util/files'; | ||||||
|
|
||||||
| export interface NamespaceInfo { | ||||||
| exportedSymbols: string[]; | ||||||
| exportedFunctions: string[]; | ||||||
| exportS3Generics: Map<string, string[]>; | ||||||
| loadsWithSideEffects: boolean; | ||||||
| } | ||||||
|
|
||||||
| export interface NAMESPACEFormat { | ||||||
|
||||||
| this: NamespaceInfo; | ||||||
|
||||||
| [packageName: string]: NamespaceInfo; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * | ||||||
| */ | ||||||
| export class FlowrNamespaceFile extends FlowrFile<NAMESPACEFormat> { | ||||||
| private readonly wrapped: FlowrFileProvider<string>; | ||||||
|
|
||||||
| /** | ||||||
| * | ||||||
| */ | ||||||
| constructor(file: FlowrFileProvider<string>) { | ||||||
| super(file.path(), file.role); | ||||||
| this.wrapped = file; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * | ||||||
| * | ||||||
|
||||||
| * | |
| * |
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly i would like lowercase here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seeing this and potential future repetition, maybe we should have a helper function for file assignments :D
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ export class FlowrAnalyzerPackageVersionsDescriptionFilePlugin extends FlowrAnal | |
| const [, name, operator, version] = match; | ||
|
|
||
| const range = Package.parsePackageVersionRange(operator, version); | ||
| ctx.deps.addDependency(new Package(name, type, undefined, range)); | ||
| ctx.deps.addDependency(new Package(name, type, undefined, undefined, range)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok this, at least for me, reaches the limit to switch from positional parameters to an object with named parameters like this: {
name,
type,
description: ...
} |
||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { FlowrAnalyzerPackageVersionsPlugin } from './flowr-analyzer-package-versions-plugin'; | ||
| import { | ||
| descriptionFileLog | ||
| } from '../file-plugins/flowr-analyzer-description-file-plugin'; | ||
| import { SemVer } from 'semver'; | ||
| import { Package } from './package'; | ||
| import type { FlowrAnalyzerContext } from '../../context/flowr-analyzer-context'; | ||
| import { SpecialFileRole } from '../../context/flowr-file'; | ||
| import type { NAMESPACEFormat } from '../file-plugins/flowr-namespace-file'; | ||
|
|
||
| /** | ||
| * | ||
| */ | ||
| export class FlowrAnalyzerPackageVersionsNamespaceFilePlugin extends FlowrAnalyzerPackageVersionsPlugin { | ||
| public readonly name = 'flowr-analyzer-package-version-namespace-file-plugin'; | ||
| public readonly description = 'This plugin does...'; | ||
| public readonly version = new SemVer('0.1.0'); | ||
|
|
||
| process(ctx: FlowrAnalyzerContext): void { | ||
| const nmspcFiles = ctx.files.getFilesByRole(SpecialFileRole.Namespace); | ||
| if(nmspcFiles.length !== 1) { | ||
| descriptionFileLog.warn(`Supporting only exactly one NAMESPACE file, found ${nmspcFiles.length}`); | ||
| return; | ||
| } | ||
|
|
||
| /** this will do the caching etc. for me */ | ||
| const deps = nmspcFiles[0].content() as NAMESPACEFormat; | ||
|
|
||
| for(const pkg in deps) { | ||
| ctx.deps.addDependency(new Package(pkg, undefined, undefined, deps[pkg])); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -142,7 +142,8 @@ function getResults(queries: readonly DependenciesQuery[], { dataflow, config, n | |
| linkedIds: linked?.length ? linked : undefined, | ||
| value: value ?? defaultValue, | ||
| versionConstraints: dep?.versionConstraints, | ||
| derivedVersion: dep?.derivedVersion | ||
| derivedVersion: dep?.derivedVersion, | ||
| namespaceInfo: dep?.namespaceInfo, | ||
|
||
| } as DependencyInfo); | ||
| if(result) { | ||
| results.push(result); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import { log } from './log'; | |
| import LineByLine from 'n-readlines'; | ||
| import type { RParseRequestFromFile } from '../r-bridge/retriever'; | ||
| import type { FlowrFileProvider } from '../project/context/flowr-file'; | ||
| import type { NAMESPACEFormat } from '../project/plugins/file-plugins/flowr-namespace-file'; | ||
|
|
||
| /** | ||
| * Represents a table, identified by a header and a list of rows. | ||
|
|
@@ -217,7 +218,78 @@ export function parseDCF(file: FlowrFileProvider<string>): Map<string, string[]> | |
| return result; | ||
| } | ||
|
|
||
| /** | ||
| * Parses the given NAMESPACE file | ||
| * @param file - The file to parse | ||
| * @returns | ||
| */ | ||
| export function parseNAMESPACE(file: FlowrFileProvider<string>): NAMESPACEFormat { | ||
| const result = { | ||
| this: { | ||
| exportedSymbols: [] as string[], | ||
| exportedFunctions: [] as string[], | ||
| exportS3Generics: new Map<string, string[]>(), | ||
| loadsWithSideEffects: false, | ||
| }, | ||
| } as NAMESPACEFormat; | ||
| const fileContent = file.content().replaceAll(cleanLineCommentRegex, '').trim() | ||
| .split(/\r?\n/).filter(Boolean); | ||
|
|
||
| for(const line of fileContent) { | ||
| const match = line.trim().match(/^(\w+)\(([^)]*)\)$/); | ||
| if(!match) { | ||
| continue; | ||
| } | ||
| const [, type, args] = match; | ||
|
|
||
| switch(type) { | ||
| case 'exportClasses': | ||
| case 'exportMethods': | ||
| result.this.exportedFunctions.push(args); | ||
| break; | ||
| case 'S3method': | ||
| { | ||
| const parts = args.split(',').map(s => s.trim()); | ||
| if(parts.length !== 2) { | ||
| continue; | ||
| } | ||
| const [pkg, func] = parts; | ||
| let arr = result.this.exportS3Generics.get(pkg); | ||
| if(!arr) { | ||
| arr = []; | ||
| result.this.exportS3Generics.set(pkg, arr); | ||
| } | ||
| arr.push(func); | ||
| break; | ||
| } | ||
| case 'export': | ||
| result.this.exportedSymbols.push(args); | ||
| break; | ||
| case 'useDynLib': | ||
| { | ||
| const parts = args.split(',').map(s => s.trim()); | ||
| if(parts.length !== 2) { | ||
| continue; | ||
| } | ||
| const [pkg, _func] = parts; | ||
|
||
| if(!result[pkg]) { | ||
| result[pkg] = { | ||
| exportedSymbols: [], | ||
| exportedFunctions: [], | ||
| exportS3Generics: new Map<string, string[]>(), | ||
| loadsWithSideEffects: false, | ||
| }; | ||
| } | ||
| result[pkg].loadsWithSideEffects = true; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how does this work? :D - the side effects part is also something that probably has to come from an external source that is unrelated to the namespace plugin but setting it here feels fair |
||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| const cleanLineCommentRegex = /^#.*$/gm; | ||
| const cleanSplitRegex = /[\n,]+/; | ||
| const cleanQuotesRegex = /'/g; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,10 +6,10 @@ describe('DESCRIPTION-file', function() { | |
| describe.sequential('Parsing', function() { | ||
| test('Library-Versions-Plugin', () => { | ||
| const p1 = new Package('Test Package'); | ||
| p1.addInfo('package', undefined, new Range('>= 1.3')); | ||
| p1.addInfo(undefined, undefined, new Range('<= 2.3')); | ||
| p1.addInfo(undefined, undefined, new Range('>= 1.5')); | ||
| p1.addInfo(undefined, undefined, new Range('<= 2.2.5')); | ||
| p1.addInfo('package', undefined, undefined, new Range('>= 1.3')); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again this should be an object with named properties, otherwise this will kill us in the future |
||
| p1.addInfo(undefined, undefined, undefined, new Range('<= 2.3')); | ||
| p1.addInfo(undefined, undefined, undefined, new Range('>= 1.5')); | ||
| p1.addInfo(undefined, undefined, undefined, new Range('<= 2.2.5')); | ||
|
|
||
| assert.isTrue(p1.derivedVersion?.test('1.7.0')); | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| import { assert, describe, test } from 'vitest'; | ||
| import path from 'path'; | ||
| import { FlowrAnalyzerContext } from '../../../../src/project/context/flowr-analyzer-context'; | ||
|
|
||
|
|
||
| import { arraysGroupBy } from '../../../../src/util/collections/arrays'; | ||
|
|
||
|
|
||
|
|
||
|
||
|
|
||
| import { FlowrInlineTextFile } from '../../../../src/project/context/flowr-file'; | ||
| import { | ||
| FlowrAnalyzerNamespaceFilePlugin | ||
| } from '../../../../src/project/plugins/file-plugins/flowr-analyzer-namespace-file-plugin'; | ||
| import { | ||
| FlowrAnalyzerPackageVersionsNamespaceFilePlugin | ||
| } from '../../../../src/project/plugins/package-version-plugins/flowr-analyzer-package-versions-namespace-file-plugin'; | ||
|
|
||
|
|
||
| describe('NAMESPACE-file', function() { | ||
| const ctx = new FlowrAnalyzerContext( | ||
| arraysGroupBy([ | ||
| new FlowrAnalyzerNamespaceFilePlugin(), | ||
| new FlowrAnalyzerPackageVersionsNamespaceFilePlugin() | ||
| ], p => p.type) | ||
| ); | ||
|
|
||
| ctx.files.addFiles(new FlowrInlineTextFile(path.resolve('NAMESPACE'), `# Generated by roxygen2 (4.0.2): do not edit by hand | ||
| S3method(as.character,expectation) | ||
| S3method(compare,character) | ||
| export(auto_test) | ||
| export(auto_test_package) | ||
| export(colourise) | ||
| export(context) | ||
| exportClasses(ListReporter) | ||
| exportClasses(MinimalReporter) | ||
| importFrom(methods,setRefClass) | ||
| useDynLib(testthat,duplicate_) | ||
| useDynLib(testthat,reassign_function)`)); | ||
|
Comment on lines
+22
to
+32
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. very nice, but for this feature, just because it is important (because we want to mine a looot of packages soon), i would like to have maybe 5-6 more real-world tests if this is ok for you |
||
| ctx.files.addRequest({ request: 'file', content: 'pete.R' }); | ||
| ctx.resolvePreAnalysis(); | ||
| describe.sequential('Parsing', function() { | ||
| test('Library-Versions-Plugin', () => { | ||
| const deps = ctx.deps.getDependency('this'); | ||
| assert.isDefined(deps); | ||
| assert.isTrue(deps.namespaceInfo?.loadsWithSideEffects === false); | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additionally, maybe we outsource:
crymove the "give me filename" thing that we also use for the description plugin into a helper function