Skip to content

Commit

Permalink
Fix binary extraction from specifier
Browse files Browse the repository at this point in the history
  • Loading branch information
webpro committed Oct 28, 2024
1 parent 229f5b3 commit d61069d
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 21 deletions.
5 changes: 3 additions & 2 deletions packages/knip/src/binaries/bash-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ import { pluginArgsMap } from '../plugins.js';
import type { GetInputsFromScriptsOptions } from '../types/config.js';
import { debugLogObject } from '../util/debug.js';
import { type Input, toBinary, toDeferResolve } from '../util/input.js';
import { extractBinary } from '../util/modules.js';
import { resolve as fallbackResolve } from './fallback.js';
import PackageManagerResolvers from './package-manager/index.js';
import { resolve as resolverFromPlugins } from './plugins.js';
import { parseNodeArgs, trimBinary } from './util.js';
import { parseNodeArgs } from './util.js';

// https://vorpaljs.github.io/bash-parser-playground/

Expand Down Expand Up @@ -35,7 +36,7 @@ export const getDependenciesFromScript = (script: string, options: GetInputsFrom
switch (node.type) {
case 'Command': {
const text = node.name?.text;
const binary = text ? trimBinary(text) : text;
const binary = text ? extractBinary(text) : text;

const commandExpansions =
node.prefix
Expand Down
3 changes: 2 additions & 1 deletion packages/knip/src/binaries/package-manager/npx.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import parseArgs from 'minimist';
import type { BinaryResolver } from '../../types/config.js';
import { toBinary, toDependency } from '../../util/input.js';
import { stripVersionFromSpecifier } from '../../util/modules.js';
import { isInternal } from '../../util/path.js';
import { argsFrom, stripVersionFromSpecifier } from '../util.js';
import { argsFrom } from '../util.js';

export const resolve: BinaryResolver = (_binary, args, options) => {
const { fromArgs } = options;
Expand Down
11 changes: 0 additions & 11 deletions packages/knip/src/binaries/util.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,5 @@
import parseArgs from 'minimist';

export const stripVersionFromSpecifier = (specifier: string) => specifier.replace(/(\S+)@.*/, '$1');

const stripNodeModulesFromPath = (command: string) => command.replace(/^(\.\/)?node_modules\//, '');

export const trimBinary = (command: string) =>
stripVersionFromSpecifier(
stripNodeModulesFromPath(command)
.replace(/^(\.bin\/)/, '')
.replace(/\$\(npm bin\)\/(\w+)/, '$1') // Removed in npm v9
);

export const argsFrom = (args: string[], from: string) => args.slice(args.indexOf(from));

export const parseNodeArgs = (args: string[]) =>
Expand Down
8 changes: 1 addition & 7 deletions packages/knip/src/util/get-referenced-inputs.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import type { ConfigurationChief, Workspace } from '../ConfigurationChief.js';
import type { DependencyDeputy } from '../DependencyDeputy.js';
import type { IssueCollector } from '../IssueCollector.js';
import { trimBinary } from '../binaries/util.js';
import { IGNORED_RUNTIME_DEPENDENCIES } from '../constants.js';
import { debugLog } from './debug.js';
import { toBinary, toDebugString } from './input.js';
import { toDebugString } from './input.js';
import { type Input, fromBinary, isBinary, isConfigPattern, isDeferResolveEntry, isDependency } from './input.js';
import { getPackageNameFromSpecifier } from './modules.js';
import { dirname, isAbsolute, isInternal, join } from './path.js';
Expand All @@ -28,11 +27,6 @@ export const getReferencedInputsHandler =

if (!containingFilePath || IGNORED_RUNTIME_DEPENDENCIES.has(specifier)) return;

if (isDeferResolveEntry(input) && specifier.includes('node_modules/.bin')) {
// TODO? this quick-fixes edge case for entry like `node node_modules/.bin/jest`, maybe do it in binaries/node.ts alone or call this function again properly to avoid this mutation
Object.assign(input, toBinary(trimBinary(input.specifier)));
}

if (isBinary(input)) {
const binaryName = fromBinary(input);
const ws = (input.dir && chief.findWorkspaceByFilePath(`${input.dir}/`)) || workspace;
Expand Down
12 changes: 12 additions & 0 deletions packages/knip/src/util/modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const getPackageNameFromModuleSpecifier = (moduleSpecifier: string) => {

const lastPackageNameMatch = /(?<=node_modules\/)(@[^/]+\/[^/]+|[^/]+)/g;
export const getPackageNameFromFilePath = (value: string) => {
if (value.includes('node_modules/.bin/')) return extractBinary(value);
const match = toPosix(value).match(lastPackageNameMatch);
if (match) return match[match.length - 1];
return value;
Expand All @@ -20,6 +21,17 @@ export const getPackageNameFromSpecifier = (specifier: string) =>

export const isStartsLikePackageName = (specifier: string) => /^(@[a-z0-9._]|[a-z0-9])/.test(specifier);

export const stripVersionFromSpecifier = (specifier: string) => specifier.replace(/(\S+)@.*/, '$1');

const stripNodeModulesFromPath = (command: string) => command.replace(/^(\.\/)?node_modules\//, '');

export const extractBinary = (command: string) =>
stripVersionFromSpecifier(
stripNodeModulesFromPath(command)
.replace(/^(\.bin\/)/, '')
.replace(/\$\(npm bin\)\/(\w+)/, '$1') // Removed in npm v9
);

export const isDefinitelyTyped = (packageName: string) => packageName.startsWith(`${DT_SCOPE}/`);

export const getDefinitelyTypedFor = (packageName: string) => {
Expand Down

0 comments on commit d61069d

Please sign in to comment.