Skip to content
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

Fix unused-files-patterns check and add tests #1050

Merged
merged 4 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
23 changes: 19 additions & 4 deletions src/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1968,7 +1968,7 @@ export async function ls(options: ILSOptions = {}): Promise<void> {
if (options.tree) {
const printableFileStructure = await util.generateFileStructureTree(
getDefaultPackageName(manifest, options),
files.map(f => ({ origin: f, tree: f }))
files.map(f => ({ origin: path.join(cwd, f), tree: f }))
);
console.log(printableFileStructure.join('\n'));
} else {
Expand Down Expand Up @@ -2011,8 +2011,23 @@ export async function printAndValidatePackagedFiles(files: IFile[], cwd: string,
// Throw an error if the extension uses the files property in package.json and
// the package does not include at least one file for each include pattern
else if (manifest.files !== undefined && manifest.files.length > 0 && !options.allowUnusedFilesPattern) {
const originalFilePaths = files.map(f => util.vsixPathToFilePath(f.path));
const unusedIncludePatterns = manifest.files.filter(includePattern => !originalFilePaths.some(filePath => minimatch(filePath, includePattern, MinimatchOptions)));
const localPaths = (files.filter(f => !isInMemoryFile(f)) as ILocalFile[]).map(f => util.normalize(f.localPath));
const filesIncludePatterns = manifest.files.map(includePattern => util.normalize(path.join(cwd, includePattern)));

const unusedIncludePatterns = filesIncludePatterns.filter(includePattern => {
// Check if the pattern provided by the user matches any file in the package
if (localPaths.some(localFilePath => minimatch(localFilePath, includePattern, MinimatchOptions))) {
return false;
}
// Check if the pattern provided by the user matches any folder in the package
if (!/(^|\/)[^/]*\*[^/]*$/.test(includePattern)) {
includePattern = (/\/$/.test(includePattern) ? `${includePattern}**` : `${includePattern}/**`);
return !localPaths.some(localFilePath => minimatch(localFilePath, includePattern, MinimatchOptions));
}
// Pattern does not match any file or folder
return true;
});

if (unusedIncludePatterns.length > 0) {
let message = '';
message += `The following include patterns in the ${chalk.bold('"files"')} property in package.json do not match any files packaged in the extension:\n`;
Expand All @@ -2030,7 +2045,7 @@ export async function printAndValidatePackagedFiles(files: IFile[], cwd: string,
getDefaultPackageName(manifest, options),
files.map(f => ({
// File path relative to the extension root
origin: util.vsixPathToFilePath(f.path),
origin: !isInMemoryFile(f) ? f.localPath : util.vsixPathToFilePath(f.path),
// File path in the VSIX
tree: f.path
})),
Expand Down
1 change: 1 addition & 0 deletions src/test/fixtures/manifestFiles/LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
LICENSE...
12 changes: 12 additions & 0 deletions src/test/fixtures/manifestFiles/foo3/bar3/hello.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Some text in here!
Some text in here!
Some text in here!
Some text in here!
Some text in here!
Some text in here!
Some text in here!
Some text in here!
Some text in here!
Some text in here!
Some text in here!
Some text in here!
2 changes: 1 addition & 1 deletion src/test/fixtures/manifestFiles/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
"publisher": "joaomoreno",
"version": "1.0.0",
"engines": { "vscode": "*" },
"files": ["foo", "foo2/bar2/include.me", "*/bar3/**", "package.json"]
"files": ["foo", "foo2/bar2/include.me", "*/bar3/**", "package.json", "LICENSE"]
}
93 changes: 92 additions & 1 deletion src/test/package.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
versionBump,
VSIX,
LicenseProcessor,
printAndValidatePackagedFiles,
} from '../package';
import { ManifestPackage } from '../manifest';
import * as path from 'path';
Expand Down Expand Up @@ -88,6 +89,63 @@ function createManifest(extra: Partial<ManifestPackage> = {}): ManifestPackage {
};
}

const PROCESS_ERROR_MESSAGE = 'PROCESS ERROR';
async function testPrintAndValidatePackagedFiles(files: IFile[], cwd: string, manifest: ManifestPackage, options: IPackageOptions, errorExpected: boolean, warningExpected: boolean): Promise<void> {
const originalLogError = log.error;
const originalLogWarn = log.warn;
const originalProcessExit = process.exit;
const warns: string[] = [];
const errors: string[] = [];
let exited = false;
let errorThrown: string | undefined;
log.error = (message: string) => errors.push(message);
log.warn = (message: string) => warns.push(message);
process.exit = (() => { exited = true; throw Error(PROCESS_ERROR_MESSAGE); }) as () => never;

try {
await printAndValidatePackagedFiles(files, cwd, manifest, options);
} catch (e: any) {
if (e instanceof Error && e.message !== PROCESS_ERROR_MESSAGE) {
errorThrown = e.message + '\n' + e.stack;
}
} finally {
process.exit = originalProcessExit;
log.error = originalLogError;
log.warn = originalLogWarn;
}

// Validate that the correct number of errors and warnings were thrown
const messages = [];

if (errorExpected !== !!errors.length) {
if (errors.length) {
messages.push(...errors);
} else {
messages.push('Expected an error');
}
}

if (warningExpected !== !!warns.length) {
if (warns.length) {
messages.push(...warns);
} else {
messages.push('Expected a warning');
}
}

if (!errorExpected && exited) {
messages.push('Process exited');
}

if (!errorExpected && !!errorThrown && !exited) {
messages.push('Error thrown: ' + errorThrown);
}

if (messages.length) {
throw new Error(messages.join('\n'));
}
}

describe('collect', function () {
this.timeout(60000);

Expand Down Expand Up @@ -133,22 +191,55 @@ describe('collect', function () {
]);
});

it('should include content of manifest.files', async () => {
it('manifest.files', async () => {
const cwd = fixture('manifestFiles');
const manifest = await readManifest(cwd);
const files = await collect(manifest, { cwd });
const names = files.map(f => f.path).sort();

await testPrintAndValidatePackagedFiles(files, cwd, manifest, {}, false, false);

assert.deepStrictEqual(names, [
'[Content_Types].xml',
'extension.vsixmanifest',
'extension/LICENSE.txt',
'extension/foo/bar/hello.txt',
'extension/foo2/bar2/include.me',
'extension/foo3/bar3/hello.txt',
'extension/package.json',
]);
});

it('manifest.files unused-files-patterns check 1', async () => {
const cwd = fixture('manifestFiles');
const manifest = await readManifest(cwd);

const manifestCopy = { ...manifest, files: [...manifest.files ?? [], 'extension/foo/bar/bye.txt'] };
const files = await collect(manifestCopy, { cwd });

await testPrintAndValidatePackagedFiles(files, cwd, manifestCopy, {}, true, false);
});

it('manifest.files unused-files-patterns check 2', async () => {
const cwd = fixture('manifestFiles');
const manifest = await readManifest(cwd);

const manifestCopy = { ...manifest, files: [...manifest.files ?? [], 'extension/fo'] };
const files = await collect(manifestCopy, { cwd });

await testPrintAndValidatePackagedFiles(files, cwd, manifestCopy, {}, true, false);
});

it('manifest.files unused-files-patterns check 3', async () => {
const cwd = fixture('manifestFiles');
const manifest = await readManifest(cwd);

const manifestCopy = { ...manifest, files: ['**'] };
const files = await collect(manifestCopy, { cwd });

await testPrintAndValidatePackagedFiles(files, cwd, manifestCopy, {}, false, false);
});

it('should ignore devDependencies', () => {
const cwd = fixture('devDependencies');
return readManifest(cwd)
Expand Down
4 changes: 4 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,10 @@ export async function generateFileStructureTree(rootFolder: string, filePaths: {
const parts = filePath.split('/');
let currentLevel = folderTree;
parts.forEach(part => {
if (currentLevel === undefined) {
throw new Error(`currentLevel is undefined for ${part} in ${filePath}`);
}

if (typeof currentLevel[part] === 'number') {
currentLevel[part] = size;
} else if (currentLevel[part]) {
Expand Down