Skip to content

Commit

Permalink
feat(project): avoid validate manifest (rome#1615)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Jul 5, 2021
1 parent e4f8983 commit a0d498a
Show file tree
Hide file tree
Showing 11 changed files with 63 additions and 28 deletions.
12 changes: 9 additions & 3 deletions internal/codec-js-manifest/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ test(
DIAGNOSTIC_CATEGORIES.parse,
"manifest",
);
t.snapshot(await normalizeManifest(PATH, manifest, []));
t.snapshot(
await normalizeManifest({path: PATH, consumer: manifest, projects: []}),
);
},
);

Expand All @@ -44,7 +46,9 @@ test(
DIAGNOSTIC_CATEGORIES.parse,
"manifest",
);
t.snapshot(await normalizeManifest(PATH, manifest, []));
t.snapshot(
await normalizeManifest({path: PATH, consumer: manifest, projects: []}),
);
},
);

Expand All @@ -63,6 +67,8 @@ test(
DIAGNOSTIC_CATEGORIES.parse,
"manifest",
);
t.snapshot(await normalizeManifest(PATH, manifest, []));
t.snapshot(
await normalizeManifest({path: PATH, consumer: manifest, projects: []}),
);
},
);
1 change: 1 addition & 0 deletions internal/codec-js-manifest/normalize/fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export function normalizeRepoField(
}
}

// TODO: review this logic
export function normalizeExportsField(
consumer: Consumer,
): boolean | ManifestExportsField {
Expand Down
31 changes: 19 additions & 12 deletions internal/codec-js-manifest/normalize/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ const INCORRECT_DEPENDENCIES_SUFFIXES = [
"dependancies",
"dependecies",
];

function checkDependencyKeyTypo(key: string, prop: Consumer) {
for (const depPrefixKey of DEPENDENCIES_KEYS) {
// Ignore if the key is a valid dependency key
Expand Down Expand Up @@ -274,7 +273,7 @@ function normalizeManifestMetadata(
metadata: ManifestMetadata;
parsedLicense: undefined | SPDXLicenseParseResult;
} {
const {consumer, loose} = context;
const {consumer, loose, checkDependenciesAndLicense} = context;
const name = normalizeNameField(consumer, loose);
const version = normalizeVersion(context);

Expand All @@ -283,12 +282,14 @@ function normalizeManifestMetadata(
}

const strName = name === undefined ? undefined : manifestNameToString(name);

const parsedLicense = normalizeLicense(
consumer,
{name: strName, version},
context,
);
let parsedLicense: undefined | SPDXLicenseParseResult = undefined;
if (checkDependenciesAndLicense) {
parsedLicense = normalizeLicense(
consumer,
{name: strName, version},
context,
);
}

return {
parsedLicense,
Expand Down Expand Up @@ -385,12 +386,18 @@ type NormalizeContext = {
loose: boolean;
consumer: Consumer;
projects: CompilerProject[];
checkDependenciesAndLicense?: boolean;
};

interface NormalizeManifest {
path: AbsoluteFilePath;
consumer: Consumer;
projects: CompilerProject[];
checkDependenciesAndLicense?: boolean;
}

export async function normalizeManifest(
path: AbsoluteFilePath,
consumer: Consumer,
projects: CompilerProject[],
{path, consumer, projects, checkDependenciesAndLicense}: NormalizeManifest,
): Promise<Manifest> {
const loose = path.hasSegment("node_modules");

Expand All @@ -399,7 +406,6 @@ export async function normalizeManifest(
for (const [key, prop] of consumer.asMap()) {
// Check for typos for dependencies
checkDependencyKeyTypo(key, prop);

// Check for other typos
const correctKey = TYPO_KEYS.get(key);
if (correctKey !== undefined) {
Expand All @@ -413,6 +419,7 @@ export async function normalizeManifest(
loose,
consumer,
projects,
checkDependenciesAndLicense,
};

const {metadata, parsedLicense} = normalizeManifestMetadata(context);
Expand Down
4 changes: 4 additions & 0 deletions internal/codec-spdx-license/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ function isLicenseValid(
const {exceptions} = parser.options;
if (exceptions !== undefined) {
for (const project of exceptions.projects) {
// if the check on dependencies is not enabled, do not do any kind of check
if (!project.config.dependencies.enabled) {
continue;
}
const {invalidLicenses} = project.config.dependencies.exceptions;
const licenses = invalidLicenses.get(id);
if (licenses === undefined) {
Expand Down
6 changes: 5 additions & 1 deletion internal/core/common/IntegrationLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,11 @@ export default class IntegrationLoader<
);
const versionProp = jsonConsumer.get("version");

const manifest = await normalizeManifest(manifestPath, jsonConsumer, []);
const manifest = await normalizeManifest({
path: manifestPath,
consumer: jsonConsumer,
projects: [],
});

if (manifest.version === undefined) {
throw versionProp.unexpected(() =>
Expand Down
8 changes: 4 additions & 4 deletions internal/core/server/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,11 @@ export default createServerCommand<Flags>({
path: manifestPath,
input: await manifestPath.readFileText(),
});
const manifestDefinition = await normalizeManifest(
manifestPath,
const manifestDefinition = await normalizeManifest({
path: manifestPath,
consumer,
[],
);
projects: [],
});
if (!manifestDefinition) {
reporter.error(markup`Couldn't find any manifest at path ${cwd}`);
return;
Expand Down
12 changes: 11 additions & 1 deletion internal/core/server/fs/MemoryFileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -703,8 +703,18 @@ export default class MemoryFileSystem {
const projects = await this.server.projectManager.getProjectHierarchyFromPath(
path,
);
let checkDependenciesAndLicense = false;
const mainProject = await this.server.projectManager.findLoadedProject(path);
if (mainProject) {
checkDependenciesAndLicense = mainProject.config.dependencies.enabled;
}
const {consumer: normalizeConsumer, diagnostics: rawDiagnostics} = consumer.capture();
const manifest = await normalizeManifest(path, normalizeConsumer, projects);
const manifest = await normalizeManifest({
path,
consumer: normalizeConsumer,
projects,
checkDependenciesAndLicense,
});

// If manifest is undefined then we failed to validate and have diagnostics
if (rawDiagnostics.length > 0) {
Expand Down
1 change: 0 additions & 1 deletion internal/core/server/fs/Resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,6 @@ function getPreferredMainKey(
if (alias !== undefined) {
return alias;
}

if (manifest.main !== undefined) {
return {
key: consumer.get("main"),
Expand Down
1 change: 0 additions & 1 deletion internal/core/server/project/ProjectManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,6 @@ export default class ProjectManager {
diagnostics: DiagnosticsProcessor,
) {
const name = manifestNameToString(def.manifest.name);

const type = isProjectPackage ? "project package manifest" : "manifest";
this.logger.info(
markup`Declaring ${type} <emphasis>${name}</emphasis> in project <emphasis>#${project.id}</emphasis> in <emphasis>${def.directory}</emphasis>`,
Expand Down
14 changes: 10 additions & 4 deletions internal/core/server/utils/getManifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,18 @@ export default async function getManifest(
const projects = await server.projectManager.getProjectHierarchyFromPath(
manifestPath,
);
let checkDependenciesAndLicense = false;
const mainProject = await server.projectManager.findLoadedProject(cwd);
if (mainProject) {
checkDependenciesAndLicense = mainProject.config.dependencies.enabled;
}
if (await manifestPath.exists()) {
manifest = await normalizeManifest(
manifestPath,
json.consumeValue(await manifestPath.readFileTextMeta()),
manifest = await normalizeManifest({
path: manifestPath,
consumer: json.consumeValue(await manifestPath.readFileTextMeta()),
projects,
);
checkDependenciesAndLicense,
});
}
return manifest;
}
1 change: 0 additions & 1 deletion internal/project/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,6 @@ export async function normalizeProjectConfig(
if (dependencies.has("enabled")) {
config.dependencies.enabled = dependencies.get("enabled").asBoolean();
}

if (dependencies.has("exceptions")) {
const exceptions = dependencies.get("exceptions").asMap();

Expand Down

0 comments on commit a0d498a

Please sign in to comment.