Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
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() ?? '');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return /^(NAMESPACE|NAMESPACE\.txt)$/i.test(file.toString().split(/[/\\]/).pop() ?? '');
return /^(NAMESPACE(\.txt)?)$/i.test(file.toString().split(/[/\\]/).pop() ?? '');

additionally, maybe we outsource:

  1. the regex to a static member
  2. cry move the "give me filename" thing that we also use for the description plugin into a helper function

}

public process(_ctx: FlowrAnalyzerContext, file: FlowrFileProvider<string>): FlowrNamespaceFile {
const f = FlowrNamespaceFile.from(file, SpecialFileRole.Namespace);
// already load it here
f.content();
Copy link
Member

Choose a reason for hiding this comment

The 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;
}
}
50 changes: 50 additions & 0 deletions src/project/plugins/file-plugins/flowr-namespace-file.ts
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 {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to have this as NamespaceFileFormat (similar to the info)

this: NamespaceInfo;
Copy link
Member

Choose a reason for hiding this comment

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

please do not use a keyword here :D

[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;
}

/**
*
*
Copy link
Member

Choose a reason for hiding this comment

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

can these be removed?

Suggested change
*
*

* @see {@link parseNAMESPACE} for details on the parsing logic.
*/
protected loadContent(): NAMESPACEFormat {
return parseNAMESPACE(this.wrapped);
Copy link
Member

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

}


/**
* Namespace file lifter, this does not re-create if already a namespace file
*/
public static from(file: FlowrFileProvider<string> | FlowrNamespaceFile, role?: SpecialFileRole): FlowrNamespaceFile {
if(role) {
file.assignRole(role);
}
Comment on lines +43 to +45
Copy link
Member

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

return file instanceof FlowrNamespaceFile ? file : new FlowrNamespaceFile(file);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Member

Choose a reason for hiding this comment

The 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: ...
}

}
}
}
Expand Down
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]));
}
}
}
12 changes: 9 additions & 3 deletions src/project/plugins/package-version-plugins/package.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Range } from 'semver';
import { guard, isNotUndefined } from '../../../util/assert';
import type { NamespaceInfo } from '../file-plugins/flowr-namespace-file';

export type PackageType = 'package' | 'system' | 'r';

Expand All @@ -8,29 +9,34 @@ export class Package {
public derivedVersion?: Range;
public type?: PackageType;
public dependencies?: Package[];
public namespaceInfo?: NamespaceInfo;
public versionConstraints: Range[] = [];

constructor(name: string, type?: PackageType, dependencies?: Package[], ...versionConstraints: readonly (Range | undefined)[]) {
constructor(name: string, type?: PackageType, dependencies?: Package[], namespaceInfo?: NamespaceInfo, ...versionConstraints: readonly (Range | undefined)[]) {
this.name = name;
this.addInfo(type, dependencies, ...(versionConstraints ?? []).filter(isNotUndefined));
this.addInfo(type, dependencies, namespaceInfo, ...(versionConstraints ?? []).filter(isNotUndefined));
}

public mergeInPlace(other: Package): void {
guard(this.name === other.name, 'Can only merge packages with the same name');
this.addInfo(
other.type,
other.dependencies,
other.namespaceInfo,
...other.versionConstraints
);
}

public addInfo(type?: PackageType, dependencies?: Package[], ...versionConstraints: readonly Range[]): void {
public addInfo(type?: PackageType, dependencies?: Package[], namespaceInfo?: NamespaceInfo, ...versionConstraints: readonly Range[]): void {
if(type !== undefined) {
this.type = type;
}
if(dependencies !== undefined) {
this.dependencies = dependencies;
}
if(namespaceInfo !== undefined) {
this.namespaceInfo = namespaceInfo;
}
if(versionConstraints !== undefined) {
this.derivedVersion ??= versionConstraints[0];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member

Choose a reason for hiding this comment

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

the indentation here feels broken

} as DependencyInfo);
if(result) {
results.push(result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { RType } from '../../../r-bridge/lang-4.x/ast/model/type';
import type { CallContextQueryResult } from '../call-context-query/call-context-query-format';
import type { Range } from 'semver';
import type { AsyncOrSync } from 'ts-essentials';
import type { NamespaceInfo } from '../../../project/plugins/file-plugins/flowr-namespace-file';

export const Unknown = 'unknown';

Expand Down Expand Up @@ -43,7 +44,8 @@ export const DefaultDependencyCategories = {
functionName: (n.info.fullLexeme ?? n.lexeme).includes(':::') ? ':::' : '::',
value: n.namespace,
versionConstraints: dep?.versionConstraints,
derivedVersion: dep?.derivedVersion
derivedVersion: dep?.derivedVersion,
namespaceInfo: dep?.namespaceInfo
});
}
});
Expand Down Expand Up @@ -92,7 +94,8 @@ export interface DependencyInfo extends Record<string, unknown>{
/** The library name, file, source, destination etc. being sourced, read from, or written to. */
value?: string
versionConstraints?: Range[],
derivedVersion?: Range
derivedVersion?: Range,
namespaceInfo?: NamespaceInfo,
}

function printResultSection(title: string, infos: DependencyInfo[], result: string[]): void {
Expand Down
72 changes: 72 additions & 0 deletions src/util/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

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

if you do not use _func just [pkg] should be fine

if(!result[pkg]) {
result[pkg] = {
exportedSymbols: [],
exportedFunctions: [],
exportS3Generics: new Map<string, string[]>(),
loadsWithSideEffects: false,
};
}
result[pkg].loadsWithSideEffects = true;
Copy link
Member

Choose a reason for hiding this comment

The 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;

Expand Down
8 changes: 4 additions & 4 deletions test/functionality/project/package.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Copy link
Member

Choose a reason for hiding this comment

The 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'));
});
Expand Down
49 changes: 49 additions & 0 deletions test/functionality/project/plugin/namespace-file.test.ts
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';



Copy link
Member

Choose a reason for hiding this comment

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

this file also contains a lot of interesting spacing 😅


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
Copy link
Member

Choose a reason for hiding this comment

The 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);
});
});
});