Skip to content

Commit

Permalink
Finalize --include-entry-exports, include scripts (closes #142)
Browse files Browse the repository at this point in the history
  • Loading branch information
webpro committed Jun 17, 2023
1 parent bd26f9c commit 1e3d3b6
Show file tree
Hide file tree
Showing 16 changed files with 96 additions and 60 deletions.
11 changes: 10 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ Using workspaces in a monorepo? Please see [workspaces][1] for more details abou
--exclude Exclude provided issue type(s) from report, can be comma-separated or repeated (1)
--dependencies Shortcut for --include dependencies,unlisted,unresolved
--exports Shortcut for --include exports,nsExports,classMembers,types,nsTypes,enumMembers,duplicates
--include-entry-exports Include entry files when reporting unused exports
--include-entry-exports Include entry files when reporting unused exports
-n, --no-progress Don't show dynamic progress updates
--reporter Select reporter: symbols, compact, codeowners, json (default: symbols)
--reporter-options Pass extra options to the reporter (as JSON string, see example)
Expand Down Expand Up @@ -579,6 +579,15 @@ export const split = function () {};

Knip does not report public exports and types as unused.

## Include exports in entry files

When a repository is self-contained or private, you may want to include entry files when reporting unused exports:

knip --include-entry-exports

Knip will also report unused exports in entry source files and scripts (such as those referenced in `package.json`). But
not in entry and configuration files from plugins, such as `next.config.js` or `src/routes/+page.svelte`.

## Handling Issues

How to handle a long list of reported issues? Seeing too many false positives? Read more about [handling issues][17]
Expand Down
10 changes: 5 additions & 5 deletions src/ProjectPrincipal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,16 +117,16 @@ export class ProjectPrincipal {
return this.extensions.has(extname(filePath));
}

public addEntryPath(filePath: string, skipExportsAnalysis = false) {
public addEntryPath(filePath: string, options?: { skipExportsAnalysis: boolean }) {
if (!isInNodeModules(filePath) && this.hasAcceptedExtension(filePath)) {
this.entryPaths.add(filePath);
this.projectPaths.add(filePath);
if (skipExportsAnalysis) this.skipExportsAnalysis.add(filePath);
if (options?.skipExportsAnalysis) this.skipExportsAnalysis.add(filePath);
}
}

public addEntryPaths(filePaths: Set<string> | string[], skipExportsAnalysis = false) {
filePaths.forEach(filePath => this.addEntryPath(filePath, skipExportsAnalysis));
public addEntryPaths(filePaths: Set<string> | string[], options?: { skipExportsAnalysis: boolean }) {
filePaths.forEach(filePath => this.addEntryPath(filePath, options));
}

public addProjectPath(filePath: string) {
Expand Down Expand Up @@ -187,7 +187,7 @@ export class ProjectPrincipal {
if (resolvedModule.isExternalLibraryImport) {
external.add(specifier);
} else {
this.addEntryPath(resolvedModule.resolvedFileName);
this.addEntryPath(resolvedModule.resolvedFileName, { skipExportsAnalysis: true });
}
} else {
if (/^(@|[a-z])/.test(specifier)) {
Expand Down
13 changes: 7 additions & 6 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
if (filePath) {
const ignorePatterns = workspace.config.ignore.map(pattern => join(dirname(containingFilePath), pattern));
const isIgnored = micromatch.isMatch(filePath, ignorePatterns);
if (!isIgnored) principal.addEntryPath(filePath, true);
if (!isIgnored) principal.addEntryPath(filePath);
} else {
collector.addIssue({ type: 'unresolved', filePath: containingFilePath, symbol: specifier });
}
Expand All @@ -106,7 +106,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
if (otherWorkspace) {
const filePath = _resolveSpecifier(otherWorkspace.dir, specifier);
if (filePath) {
principal.addEntryPath(filePath);
principal.addEntryPath(filePath, { skipExportsAnalysis: true });
} else {
collector.addIssue({ type: 'unresolved', filePath: containingFilePath, symbol: specifier });
}
Expand Down Expand Up @@ -168,7 +168,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
const patterns = worker.getProductionPluginEntryFilePatterns();
const pluginWorkspaceEntryPaths = await _glob({ ...sharedGlobOptions, patterns });
debugLogArray(`Found production plugin entry paths (${name})`, pluginWorkspaceEntryPaths);
principal.addEntryPaths(pluginWorkspaceEntryPaths, true);
principal.addEntryPaths(pluginWorkspaceEntryPaths, { skipExportsAnalysis: true });
}

{
Expand Down Expand Up @@ -196,7 +196,7 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
const patterns = worker.getPluginEntryFilePatterns();
const pluginWorkspaceEntryPaths = await _glob({ ...sharedGlobOptions, patterns });
debugLogArray(`Found plugin entry paths (${name})`, pluginWorkspaceEntryPaths);
principal.addEntryPaths(pluginWorkspaceEntryPaths, true);
principal.addEntryPaths(pluginWorkspaceEntryPaths, { skipExportsAnalysis: true });
}

{
Expand All @@ -210,12 +210,13 @@ export const main = async (unresolvedConfiguration: CommandLineOptions) => {
const patterns = compact(worker.getPluginConfigPatterns());
const configurationEntryPaths = await _glob({ ...sharedGlobOptions, patterns });
debugLogArray(`Found plugin configuration paths (${name})`, configurationEntryPaths);
principal.addEntryPaths(configurationEntryPaths, true);
principal.addEntryPaths(configurationEntryPaths, { skipExportsAnalysis: true });
}
}

// Add knip.ts (might import dependencies)
if (chief.resolvedConfigFilePath) principal.addEntryPath(chief.resolvedConfigFilePath, true);
if (chief.resolvedConfigFilePath)
principal.addEntryPath(chief.resolvedConfigFilePath, { skipExportsAnalysis: true });

// Get peerDependencies, installed binaries, entry files gathered through all plugins, and hand over
// A bit of an entangled hotchpotch, but it's all related, and efficient in terms of reading package.json once, etc.
Expand Down
2 changes: 1 addition & 1 deletion src/util/cli-arguments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Options:
--exclude Exclude provided issue type(s) from report, can be comma-separated or repeated (1)
--dependencies Shortcut for --include dependencies,unlisted,unresolved
--exports Shortcut for --include exports,nsExports,classMembers,types,nsTypes,enumMembers,duplicates
--include-entry-exports Include entry files when reporting unused exports
--include-entry-exports Include entry files when reporting unused exports
-n, --no-progress Don't show dynamic progress updates
--reporter Select reporter: symbols, compact, codeowners, json (default: symbols)
--reporter-options Pass extra options to the reporter (as JSON string, see example)
Expand Down
22 changes: 0 additions & 22 deletions tests/exports-plugin-entry-exports.test.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = () => {};
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@
"type": "module",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1",
"module1": "tsx src/module1/index.ts",
"module2": "tsx src/module2/index.ts",
"module3": "tsx src/module3/index.ts"
"module1": "tsx ./src/script1.ts",
"module2": "tsx ./src/script2.ts",
"module3": "tsx ./src/script3.ts"
},
"keywords": [],
"author": "",
"license": "ISC",
"dependencies": {
"next": "*"
},
"devDependencies": {
"knip": "2.13.0",
"tsx": "3.12.7",
Expand Down
13 changes: 13 additions & 0 deletions tests/fixtures/include-entry-exports-scripts/src/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import Head from 'next/head';

function IndexPage() {
return (
<div>
<Head>
<title>My page title</title>
</Head>
</div>
);
}

export default IndexPage;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { subtract } from '../module2/index.js';
import { subtract } from './script2.js';

// this is NOT reported by knip, because we don't import anything throughout the app from 'module3/index.ts'
export function calculate(a: number, b: number) {
Expand Down
Empty file.
37 changes: 37 additions & 0 deletions tests/include-entry-exports-scripts.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import { main } from '../src/index.js';
import { resolve } from '../src/util/path.js';
import baseArguments from './helpers/baseArguments.js';
import baseCounters from './helpers/baseCounters.js';

const cwd = resolve('tests/fixtures/include-entry-exports-scripts');

test('Skip unused exports in entry source files and scripts', async () => {
const { counters } = await main({
...baseArguments,
cwd,
isIncludeEntryExports: false,
});

assert.deepEqual(counters, {
...baseCounters,
processed: 5,
total: 5,
});
});

test('Report unused exports in source files and scripts (skip for plugin entry files)', async () => {
const { counters } = await main({
...baseArguments,
cwd,
isIncludeEntryExports: true,
});

assert.deepEqual(counters, {
...baseCounters,
exports: 5,
processed: 5,
total: 5,
});
});
36 changes: 15 additions & 21 deletions tests/include-entry-exports.test.ts
Original file line number Diff line number Diff line change
@@ -1,48 +1,42 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import { main } from '../src/index.js';
import { resolve, join } from '../src/util/path.js';
import { resolve } from '../src/util/path.js';
import baseArguments from './helpers/baseArguments.js';
import baseCounters from './helpers/baseCounters.js';

const cwd = resolve('tests/fixtures/include-entry-exports');

test('Report unused files and exports in entry files', async () => {
const { issues, counters } = await main({
test('Skip unused exports in entry source files', async () => {
const { counters } = await main({
...baseArguments,
cwd,
isIncludeEntryExports: true,
isIncludeEntryExports: false,
});

assert(issues.files.has(join(cwd, 'unused.ts')));

assert(issues.exports['cli.js']['a']);
assert(issues.exports['index.ts']['default']);
assert(issues.exports['main.ts']['x']);

assert.deepEqual(counters, {
...baseCounters,
files: 1,
exports: 3,
types: 3,
processed: 5,
total: 5,
processed: 4,
total: 4,
});
});

test('Skip unused files and exports in entry files', async () => {
test('Report unused exports in entry source files', async () => {
const { issues, counters } = await main({
...baseArguments,
cwd,
isIncludeEntryExports: false,
isIncludeEntryExports: true,
});

assert(issues.files.has(join(cwd, 'unused.ts')));
assert(issues.exports['cli.js']['a']);
assert(issues.exports['index.ts']['default']);
assert(issues.exports['main.ts']['x']);

assert.deepEqual(counters, {
...baseCounters,
files: 1,
processed: 5,
total: 5,
exports: 3,
types: 3,
processed: 4,
total: 4,
});
});

0 comments on commit 1e3d3b6

Please sign in to comment.