diff --git a/desloppify/languages/_framework/treesitter/analysis/unused_imports.py b/desloppify/languages/_framework/treesitter/analysis/unused_imports.py index 666f694d1..a304be6da 100644 --- a/desloppify/languages/_framework/treesitter/analysis/unused_imports.py +++ b/desloppify/languages/_framework/treesitter/analysis/unused_imports.py @@ -7,6 +7,7 @@ from __future__ import annotations import logging +import os import re from typing import TYPE_CHECKING @@ -21,6 +22,22 @@ _ECMASCRIPT_IMPORT_NODE_TYPE = "import_statement" +# Symbols a sourced Bash file defines for the sourcing script: functions +# (both `name() {}` and `function name {}` forms, global regardless of +# nesting) and top-level variable assignments (including `export`/`declare` +# wrappers). Function-local `local`/`declare` assignments are excluded. +_BASH_DEFINITION_QUERY = """ + (function_definition + name: (word) @name) + (program + (variable_assignment + name: (variable_name) @name)) + (program + (declaration_command + (variable_assignment + name: (variable_name) @name))) +""" + # Identifier-ish nodes that represent a reference to a binding in JavaScript/TypeScript. # JSX tag names are typically represented as `identifier` in tree-sitter-javascript/tsx, # but we include `jsx_identifier` as well for compatibility with grammar variants. @@ -72,6 +89,7 @@ def detect_unused_imports( query = _make_query(language, spec.import_query) entries: list[dict] = [] + sourced_symbol_cache: dict[str, frozenset[str]] = {} for filepath in file_list: cached = get_or_parse_tree(filepath, parser, spec.grammar) @@ -126,6 +144,21 @@ def detect_unused_imports( if not name: continue + # Bash `source`/`.` loads a library whose *defined* functions and + # variables are what the sourcing script uses — the sourced file's + # basename almost never reappears in the script body. Treat the + # import as used when any symbol the library defines is referenced. + if spec.grammar == "bash": + symbols = _bash_sourced_symbols( + raw_path, filepath, spec, parser, language, + sourced_symbol_cache, + ) + if any( + re.search(r'\b' + re.escape(symbol) + r'\b', rest) + for symbol in symbols + ): + continue + # Check if the name appears in the rest of the file. if not re.search(r'\b' + re.escape(name) + r'\b', rest): entries.append({ @@ -137,6 +170,54 @@ def detect_unused_imports( return entries +def _bash_sourced_symbols( + raw_path: str, + filepath: str, + spec: TreeSitterLangSpec, + parser, + language, + cache: dict[str, frozenset[str]], +) -> frozenset[str]: + """Extract the function/variable names a sourced Bash file defines. + + Resolves ``raw_path`` relative to the sourcing script. Returns an empty + set when the sourced file cannot be resolved (variable-based paths, + missing files) so the caller falls back to the basename heuristic. + """ + resolver = spec.resolve_import + if resolver is None: + return frozenset() + try: + resolved = resolver(raw_path, filepath, os.path.dirname(filepath)) + except (OSError, ValueError): + return frozenset() + if not resolved: + return frozenset() + + resolved = os.path.abspath(resolved) + cached_symbols = cache.get(resolved) + if cached_symbols is not None: + return cached_symbols + + symbols: frozenset[str] = frozenset() + parsed = get_or_parse_tree(resolved, parser, spec.grammar) + if parsed is not None: + _source, tree = parsed + definition_query = _make_query(language, _BASH_DEFINITION_QUERY) + names: set[str] = set() + for _pattern_idx, captures in _run_query(definition_query, tree.root_node): + name_node = _unwrap_node(captures.get("name")) + if name_node is None: + continue + text = _node_text(name_node) + if text: + names.add(text) + symbols = frozenset(names) + + cache[resolved] = symbols + return symbols + + def _detect_unused_imports_ecmascript( file_list: list[str], spec: TreeSitterLangSpec, diff --git a/desloppify/tests/lang/common/test_bash_unused_imports.py b/desloppify/tests/lang/common/test_bash_unused_imports.py index c74dee941..a7447b180 100644 --- a/desloppify/tests/lang/common/test_bash_unused_imports.py +++ b/desloppify/tests/lang/common/test_bash_unused_imports.py @@ -5,17 +5,34 @@ import textwrap -def _detect(tmp_path, contents: str): +def _detect(tmp_path, contents: str, libraries: dict[str, str] | None = None): from desloppify.languages._framework.treesitter.analysis.unused_imports import ( detect_unused_imports, ) from desloppify.languages._framework.treesitter.specs.scripting import BASH_SPEC + for lib_name, lib_contents in (libraries or {}).items(): + (tmp_path / lib_name).write_text(textwrap.dedent(lib_contents).lstrip()) script = tmp_path / "script.sh" script.write_text(textwrap.dedent(contents).lstrip()) return detect_unused_imports([str(script)], BASH_SPEC) +_LIB_ACCEPTANCE = """ + #!/usr/bin/env bash + API=${API:-http://localhost:8080} + export TOKEN="abc" + declare -r LIMIT=5 + function legacy_helper { + local inner=1 + echo "$inner" + } + require_server() { + curl -fsS "$API/health" >/dev/null + } +""" + + def test_bash_shell_flags_are_not_imports(tmp_path): findings = _detect( tmp_path, @@ -86,3 +103,90 @@ def test_bash_used_source_directive_is_not_flagged(tmp_path): ) assert findings == [] + + +def test_bash_source_calling_library_function_is_not_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + #!/bin/bash + set -euo pipefail + cd "$(dirname "$0")" + source ./lib-acceptance.sh + require_server + """, + libraries={"lib-acceptance.sh": _LIB_ACCEPTANCE}, + ) + + assert findings == [] + + +def test_bash_source_using_function_keyword_definition_is_not_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + #!/bin/bash + source ./lib-acceptance.sh + legacy_helper + """, + libraries={"lib-acceptance.sh": _LIB_ACCEPTANCE}, + ) + + assert findings == [] + + +def test_bash_source_using_library_variable_is_not_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + #!/bin/bash + source ./lib-acceptance.sh + curl -fsS "$API/workflows" >/dev/null + """, + libraries={"lib-acceptance.sh": _LIB_ACCEPTANCE}, + ) + + assert findings == [] + + +def test_bash_source_using_exported_variable_is_not_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + #!/bin/bash + source ./lib-acceptance.sh + echo "token: ${TOKEN}" + """, + libraries={"lib-acceptance.sh": _LIB_ACCEPTANCE}, + ) + + assert findings == [] + + +def test_bash_source_with_no_library_symbol_used_is_flagged(tmp_path): + findings = _detect( + tmp_path, + """ + #!/bin/bash + source ./lib-acceptance.sh + echo body + """, + libraries={"lib-acceptance.sh": _LIB_ACCEPTANCE}, + ) + + assert [entry["name"] for entry in findings] == ["lib-acceptance"] + + +def test_bash_source_local_variable_in_library_function_is_not_usage(tmp_path): + findings = _detect( + tmp_path, + """ + #!/bin/bash + source ./lib-acceptance.sh + inner=2 + echo "$inner" + """, + libraries={"lib-acceptance.sh": _LIB_ACCEPTANCE}, + ) + + assert [entry["name"] for entry in findings] == ["lib-acceptance"]