Skip to content

Commit

Permalink
fix unused-files-patterns check and add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
benibenj committed Sep 4, 2024
1 parent 0ec0e7f commit 134eaa6
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 6 deletions.
23 changes: 19 additions & 4 deletions src/package.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1955,7 +1955,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 @@ -1998,8 +1998,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 @@ -2017,7 +2032,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 : path.join(cwd, 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"]
}
66 changes: 65 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,36 @@ function createManifest(extra: Partial<ManifestPackage> = {}): ManifestPackage {
};
}

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[] = [];
log.error = (message: string) => errors.push(message);
log.warn = (message: string) => warns.push(message);
process.exit = (() => { throw Error('Error'); }) as () => never;

let exitedOrErrorThrown = false;
try {
await printAndValidatePackagedFiles(files, cwd, manifest, options);
} catch (e) {
exitedOrErrorThrown = true;
} finally {
process.exit = originalProcessExit;
log.error = originalLogError;
log.warn = originalLogWarn;
}

if (errorExpected !== !!errors.length || exitedOrErrorThrown !== errorExpected) {
throw new Error(errors.length ? errors.join('\n') : 'Expected error');
}

if (warningExpected !== !!warns.length) {
throw new Error(warns.length ? warns.join('\n') : 'Expected warning');
}
}

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

Expand Down Expand Up @@ -133,22 +164,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

0 comments on commit 134eaa6

Please sign in to comment.