From 8b798afae2811aba8c15b280542a5af680369caa Mon Sep 17 00:00:00 2001 From: Lars Kappert Date: Wed, 23 Aug 2023 19:34:32 +0200 Subject: [PATCH] Revert "Migrate from bash-parser to tree-sitter (closes #72)" This reverts commit ef3981f9a8386088db148f2ecd23348e69500381. --- src/binaries/bash-parser.ts | 110 +++++++++--------- src/binaries/resolvers/npx.ts | 3 +- src/binaries/resolvers/rollup.ts | 3 +- src/typescript/ast-helpers.ts | 23 +++- .../visitors/exports/exportKeyword.ts | 3 +- .../exports/moduleExportsAccessExpression.ts | 3 +- src/typescript/visitors/scripts/execa.ts | 2 +- src/typescript/visitors/scripts/zx.ts | 2 +- src/util/string.ts | 21 ---- tests/util/getReferencesFromScripts.test.ts | 6 +- 10 files changed, 86 insertions(+), 90 deletions(-) delete mode 100644 src/util/string.ts diff --git a/src/binaries/bash-parser.ts b/src/binaries/bash-parser.ts index fa79c4203..a4c237f50 100644 --- a/src/binaries/bash-parser.ts +++ b/src/binaries/bash-parser.ts @@ -1,77 +1,77 @@ -import Parser from 'tree-sitter'; -import Bash from 'tree-sitter-bash'; +import parse from '@ericcornelissen/bash-parser'; import { debugLogObject } from '../util/debug.js'; import * as FallbackResolver from './resolvers/fallback.js'; import * as KnownResolvers from './resolvers/index.js'; import { stripBinaryPath } from './util.js'; +import type { Node } from '@ericcornelissen/bash-parser'; import type { PackageJson } from '@npmcli/package-json'; -import type { SyntaxNode } from 'tree-sitter'; -type KnownResolver = keyof typeof KnownResolvers; - -const parser = new Parser(); -parser.setLanguage(Bash); - -const getCommandsFromScript = (script: string) => { - const tree = parser.parse(script); - const commands: string[][] = []; - - const traverse = (node: SyntaxNode) => { - switch (node.type) { - case 'command': { - const commandNameIndex = node.children.findIndex(node => node.type === 'command_name'); - const command = node.children.slice(commandNameIndex).map(node => node.text); - commands.push(command); - break; - } - default: - break; - } - - for (const child of node.children) { - traverse(child); - } - }; +// https://vorpaljs.github.io/bash-parser-playground/ - traverse(tree.rootNode); - - return commands; -}; +type KnownResolver = keyof typeof KnownResolvers; export const getBinariesFromScript = ( script: string, { cwd, manifest, knownGlobalsOnly = false }: { cwd: string; manifest: PackageJson; knownGlobalsOnly?: boolean } -): string[] => { +) => { if (!script) return []; // Helper for recursive calls const fromArgs = (args: string[]) => getBinariesFromScript(args.filter(arg => arg !== '--').join(' '), { cwd, manifest }); - const commands = getCommandsFromScript(script); - - const getBinariesFromCommand = (command: string[]) => { - const [bin, ...args] = command; - const binary = stripBinaryPath(bin); - - if (!binary || binary === '.' || binary === 'source') return []; - if (binary.startsWith('-') || binary.startsWith('"') || binary.startsWith('..')) return []; - if (['bun', 'deno'].includes(binary)) return []; - - if (binary in KnownResolvers) { - return KnownResolvers[binary as KnownResolver].resolve(binary, args, { cwd, manifest, fromArgs }); - } - - // Before using the fallback resolver, we need a way to bail out for scripts in environments like GitHub - // Actions, which are provisioned with lots of unknown global binaries. - if (knownGlobalsOnly) return []; - - // We apply a kitchen sink fallback resolver for everything else - return FallbackResolver.resolve(binary, args, { cwd, manifest, fromArgs }); - }; + const getBinariesFromNodes = (nodes: Node[]): string[] => + nodes.flatMap(node => { + switch (node.type) { + case 'Command': { + const binary = node.name?.text ? stripBinaryPath(node.name.text) : node.name?.text; + + const commandExpansions = + node.prefix?.flatMap( + prefix => prefix.expansion?.filter(expansion => expansion.type === 'CommandExpansion') ?? [] + ) ?? []; + + if (commandExpansions.length > 0) { + return commandExpansions.flatMap(expansion => getBinariesFromNodes(expansion.commandAST.commands)) ?? []; + } + + // Bunch of early bail outs for things we can't or don't want to resolve + if (!binary || binary === '.' || binary === 'source') return []; + if (binary.startsWith('-') || binary.startsWith('"') || binary.startsWith('..')) return []; + if (['bun', 'deno'].includes(binary)) return []; + + const args = node.suffix?.map(arg => arg.text) ?? []; + + // Commands that precede other commands, try again with the rest + if (['!', 'test'].includes(binary)) return fromArgs(args); + + if (binary in KnownResolvers) { + return KnownResolvers[binary as KnownResolver].resolve(binary, args, { cwd, manifest, fromArgs }); + } + + // Before using the fallback resolver, we need a way to bail out for scripts in environments like GitHub + // Actions, which are provisioned with lots of unknown global binaries. + if (knownGlobalsOnly) return []; + + // We apply a kitchen sink fallback resolver for everything else + return FallbackResolver.resolve(binary, args, { cwd, manifest, fromArgs }); + } + case 'LogicalExpression': + return getBinariesFromNodes([node.left, node.right]); + case 'If': + return getBinariesFromNodes([...node.clause.commands, ...node.then.commands, ...(node.else?.commands ?? [])]); + case 'For': + return getBinariesFromNodes(node.do.commands); + case 'CompoundList': + return getBinariesFromNodes(node.commands); + default: + return []; + } + }); try { - return commands.map(getBinariesFromCommand).flat(); + const parsed = parse(script); + return parsed?.commands ? getBinariesFromNodes(parsed.commands) : []; } catch (error) { debugLogObject('Bash parser error', error); return []; diff --git a/src/binaries/resolvers/npx.ts b/src/binaries/resolvers/npx.ts index 98a8fe8d3..c8780de99 100644 --- a/src/binaries/resolvers/npx.ts +++ b/src/binaries/resolvers/npx.ts @@ -1,6 +1,5 @@ import parseArgs from 'minimist'; import { isInternal } from '../../util/path.js'; -import { stripQuotes } from '../../util/string.js'; import { getBinariesFromScript } from '../bash-parser.js'; import { argsFrom, stripVersionFromSpecifier, toBinary } from '../util.js'; import type { Resolver } from '../types.js'; @@ -15,7 +14,7 @@ export const resolve: Resolver = (binary, args, { cwd, fromArgs, manifest }) => const specifier = packageSpecifier ? stripVersionFromSpecifier(packageSpecifier) : ''; const packages = parsed.package ? [parsed.package].flat().map(stripVersionFromSpecifier) : []; - const command = parsed.call ? getBinariesFromScript(stripQuotes(parsed.call), { cwd, manifest }) : []; + const command = parsed.call ? getBinariesFromScript(parsed.call, { cwd, manifest }) : []; const restArgs = argsFrom(args, packageSpecifier); const dependencies = manifest ? Object.keys({ ...manifest.dependencies, ...manifest.devDependencies }) : []; diff --git a/src/binaries/resolvers/rollup.ts b/src/binaries/resolvers/rollup.ts index 288ab3ddc..899b71541 100644 --- a/src/binaries/resolvers/rollup.ts +++ b/src/binaries/resolvers/rollup.ts @@ -1,6 +1,5 @@ import parseArgs from 'minimist'; import { compact } from '../../util/array.js'; -import { stripQuotes } from '../../util/string.js'; import { toBinary, tryResolveSpecifiers } from '../util.js'; import type { Resolver } from '../types.js'; @@ -8,7 +7,7 @@ export const resolve: Resolver = (binary, args, { cwd, fromArgs }) => { // minimist throws when `--watch` is followed by other dotted `--watch.*` arguments const safeArgs = args.filter(arg => arg !== '--watch'); const parsed = parseArgs(safeArgs, { alias: { plugin: 'p' } }); - const watchers = parsed.watch ? fromArgs(Object.values(parsed.watch).map(value => stripQuotes(value))) : []; + const watchers = parsed.watch ? fromArgs(Object.values(parsed.watch)) : []; const plugins = parsed.plugin ? tryResolveSpecifiers(cwd, [parsed.plugin].flat()) : []; const configPlugins = parsed.configPlugin ? tryResolveSpecifiers(cwd, [parsed.configPlugin].flat()) : []; return compact([toBinary(binary), ...watchers, ...plugins, ...configPlugins]); diff --git a/src/typescript/ast-helpers.ts b/src/typescript/ast-helpers.ts index ecec9180c..fae5a506c 100644 --- a/src/typescript/ast-helpers.ts +++ b/src/typescript/ast-helpers.ts @@ -1,5 +1,4 @@ import ts from 'typescript'; -import { stripQuotes } from '../util/string.js'; interface ValidImportTypeNode extends ts.ImportTypeNode { argument: ts.LiteralTypeNode & { literal: ts.StringLiteral }; @@ -71,6 +70,28 @@ export function isModuleExportsAccessExpression( ); } +export function stripQuotes(name: string) { + const length = name.length; + if (length >= 2 && name.charCodeAt(0) === name.charCodeAt(length - 1) && isQuoteOrBacktick(name.charCodeAt(0))) { + return name.substring(1, length - 1); + } + return name; +} + +const enum CharacterCodes { + backtick = 0x60, + doubleQuote = 0x22, + singleQuote = 0x27, +} + +function isQuoteOrBacktick(charCode: number) { + return ( + charCode === CharacterCodes.singleQuote || + charCode === CharacterCodes.doubleQuote || + charCode === CharacterCodes.backtick + ); +} + export function findAncestor( node: ts.Node | undefined, callback: (element: ts.Node) => boolean | 'STOP' diff --git a/src/typescript/visitors/exports/exportKeyword.ts b/src/typescript/visitors/exports/exportKeyword.ts index 9ff21564c..a9511f52f 100644 --- a/src/typescript/visitors/exports/exportKeyword.ts +++ b/src/typescript/visitors/exports/exportKeyword.ts @@ -1,8 +1,7 @@ import ts from 'typescript'; import { SymbolType } from '../../../types/issues.js'; import { compact } from '../../../util/array.js'; -import { stripQuotes } from '../../../util/string.js'; -import { isPrivateMember } from '../../ast-helpers.js'; +import { isPrivateMember, stripQuotes } from '../../ast-helpers.js'; import { exportVisitor as visit } from '../index.js'; export default visit( diff --git a/src/typescript/visitors/exports/moduleExportsAccessExpression.ts b/src/typescript/visitors/exports/moduleExportsAccessExpression.ts index 4a96e122d..709147509 100644 --- a/src/typescript/visitors/exports/moduleExportsAccessExpression.ts +++ b/src/typescript/visitors/exports/moduleExportsAccessExpression.ts @@ -1,7 +1,6 @@ import ts from 'typescript'; import { SymbolType } from '../../../types/issues.js'; -import { stripQuotes } from '../../../util/string.js'; -import { isModuleExportsAccessExpression } from '../../ast-helpers.js'; +import { isModuleExportsAccessExpression, stripQuotes } from '../../ast-helpers.js'; import { isJS } from '../helpers.js'; import { exportVisitor as visit } from '../index.js'; diff --git a/src/typescript/visitors/scripts/execa.ts b/src/typescript/visitors/scripts/execa.ts index 495cf4d2d..54ef23272 100644 --- a/src/typescript/visitors/scripts/execa.ts +++ b/src/typescript/visitors/scripts/execa.ts @@ -1,5 +1,5 @@ import ts from 'typescript'; -import { stripQuotes } from '../../../util/string.js'; +import { stripQuotes } from '../../ast-helpers.js'; import { scriptVisitor as visit } from '../index.js'; export default visit( diff --git a/src/typescript/visitors/scripts/zx.ts b/src/typescript/visitors/scripts/zx.ts index bed5cb237..85605ed31 100644 --- a/src/typescript/visitors/scripts/zx.ts +++ b/src/typescript/visitors/scripts/zx.ts @@ -1,5 +1,5 @@ import ts from 'typescript'; -import { stripQuotes } from '../../../util/string.js'; +import { stripQuotes } from '../../ast-helpers.js'; import { scriptVisitor as visit } from '../index.js'; export default visit( diff --git a/src/util/string.ts b/src/util/string.ts deleted file mode 100644 index ca73001a2..000000000 --- a/src/util/string.ts +++ /dev/null @@ -1,21 +0,0 @@ -export function stripQuotes(name: string) { - const length = name.length; - if (length >= 2 && name.charCodeAt(0) === name.charCodeAt(length - 1) && isQuoteOrBacktick(name.charCodeAt(0))) { - return name.substring(1, length - 1); - } - return name; -} - -const enum CharacterCodes { - backtick = 0x60, - doubleQuote = 0x22, - singleQuote = 0x27, -} - -function isQuoteOrBacktick(charCode: number) { - return ( - charCode === CharacterCodes.singleQuote || - charCode === CharacterCodes.doubleQuote || - charCode === CharacterCodes.backtick - ); -} diff --git a/tests/util/getReferencesFromScripts.test.ts b/tests/util/getReferencesFromScripts.test.ts index 59f9f45be..54e2f98c5 100644 --- a/tests/util/getReferencesFromScripts.test.ts +++ b/tests/util/getReferencesFromScripts.test.ts @@ -63,7 +63,7 @@ test('getReferencesFromScripts (--require)', () => { test('getReferencesFromScripts (.bin)', () => { t('./node_modules/.bin/tsc --noEmit', ['bin:tsc']); t('node_modules/.bin/tsc --noEmit', ['bin:tsc']); - t('$(npm bin)/tsc --noEmit', ['bin:tsc', 'bin:npm']); + t('$(npm bin)/tsc --noEmit', ['bin:tsc']); t('../../../scripts/node_modules/.bin/tsc --noEmit', []); }); @@ -162,7 +162,7 @@ test('getReferencesFromScripts (c8)', () => { }); test('getReferencesFromScripts (bash expressions)', () => { - t('if test "$NODE_ENV" = "production" ; then make install ; fi ', ['bin:test', 'bin:make']); + t('if test "$NODE_ENV" = "production" ; then make install ; fi ', ['bin:make']); t('node -e "if (NODE_ENV === \'production\'){process.exit(1)} " || make install', ['bin:make']); t('if ! npx pkg --verbose ; then exit 1 ; fi', ['bin:pkg', 'bin:exit']); t('exec < /dev/tty && node_modules/.bin/cz --hook || true', ['bin:exec', 'bin:cz', 'bin:true']); @@ -174,7 +174,7 @@ test('getReferencesFromScripts (bash expansion)', () => { }); test('getReferencesFromScripts (multiline)', () => { - t('#!/bin/sh\n. "$(dirname "$0")/_/husky.sh"\nnpx lint-staged', ['bin:dirname', 'bin:lint-staged']); + t('#!/bin/sh\n. "$(dirname "$0")/_/husky.sh"\nnpx lint-staged', ['bin:lint-staged']); t(`for S in "s"; do\n\tnpx rc@0.6.0\n\tnpx @scope/rc@0.6.0\ndone`, ['rc', '@scope/rc']); });