Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CLI commands for detecting and remove dead code #516

Merged
merged 6 commits into from Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions README.md
Expand Up @@ -341,6 +341,22 @@ require "spoom/backtrace_filter/minitest"
Minitest.backtrace_filter = Spoom::BacktraceFilter::Minitest.new
```

### Dead code removal

Run dead code detection in your project with:

```
$ spoom deadcode
```

This will list all the methods and constants that do not appear to be used in your project.

You can remove them with Spoom:

```
$ spoom deadcode remove path/to/file.rb:42:18-47:23
```

## Development

After checking out the repo, run `bin/setup` to install dependencies. Then, run `bin/test` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. Don't forget to run `bin/sanity` before pushing your changes.
Expand Down
4 changes: 4 additions & 0 deletions lib/spoom/cli.rb
Expand Up @@ -6,6 +6,7 @@
require_relative "cli/helper"

require_relative "cli/bump"
require_relative "cli/deadcode"
require_relative "cli/lsp"
require_relative "cli/coverage"
require_relative "cli/run"
Expand All @@ -27,6 +28,9 @@ class Main < Thor
desc "coverage", "Collect metrics related to Sorbet coverage"
subcommand "coverage", Spoom::Cli::Coverage

desc "deadcode", "Analyze code to find deadcode"
subcommand "deadcode", Spoom::Cli::Deadcode

desc "lsp", "Send LSP requests to Sorbet"
subcommand "lsp", Spoom::Cli::LSP

Expand Down
172 changes: 172 additions & 0 deletions lib/spoom/cli/deadcode.rb
@@ -0,0 +1,172 @@
# typed: true
# frozen_string_literal: true

require_relative "../deadcode"

module Spoom
module Cli
class Deadcode < Thor
extend T::Sig
include Helper

default_task :deadcode

desc "deadcode PATH...", "Analyze PATHS to find dead code"
option :allowed_extensions,
type: :array,
default: [".rb", ".erb", ".gemspec"],
aliases: :e,
desc: "Allowed extensions"
option :allowed_mime_types,
type: :array,
default: ["text/x-ruby", "text/x-ruby-script"],
aliases: :m,
desc: "Allowed mime types"
option :exclude,
type: :array,
default: ["vendor/", "sorbet/"],
aliases: :x,
desc: "Exclude paths"
option :show_files,
type: :boolean,
default: false,
desc: "Show the files that will be analyzed"
option :show_plugins,
type: :boolean,
default: false,
desc: "Show the loaded plugins"
option :show_defs,
type: :boolean,
default: false,
desc: "Show the indexed definitions"
option :show_refs,
type: :boolean,
default: false,
desc: "Show the indexed references"
option :sort,
type: :string,
default: "name",
enum: ["name", "location"],
desc: "Sort the output by name or location"
sig { params(paths: String).void }
def deadcode(*paths)
context = self.context

paths << exec_path if paths.empty?

$stderr.puts "Collecting files..."
collector = FileCollector.new(
allow_extensions: options[:allowed_extensions],
allow_mime_types: options[:allowed_mime_types],
exclude_patterns: options[:exclude].map { |p| Pathname.new(File.join(exec_path, p, "**")).cleanpath.to_s },
)
collector.visit_paths(paths)
files = collector.files.sort

if options[:show_files]
$stderr.puts "\nCollected #{blue(files.size.to_s)} files for analysis\n"
files.each do |file|
$stderr.puts " #{gray(file)}"
end
$stderr.puts
end

plugins = Spoom::Deadcode.plugins_from_gemfile_lock(context)
if options[:show_plugins]
$stderr.puts "\nLoaded #{blue(plugins.size.to_s)} plugins\n"
plugins.each do |plugin|
$stderr.puts " #{gray(plugin.class.to_s)}"
end
$stderr.puts
end

index = Spoom::Deadcode::Index.new

$stderr.puts "Indexing #{blue(files.size.to_s)} files..."
files.each do |file|
content = File.read(file)
content = ERB.new(content).src if file.end_with?(".erb")

tree = Spoom::Deadcode.parse_ruby(content, file: file)
Spoom::Deadcode.index_node(index, tree, content, file: file, plugins: plugins)
rescue Spoom::Deadcode::ParserError => e
say_error("Error parsing #{file}: #{e.message}")
next
rescue Spoom::Deadcode::IndexerError => e
say_error("Error indexing #{file}: #{e.message}")
next
end

if options[:show_defs]
$stderr.puts "\nDefinitions:"
index.definitions.each do |name, definitions|
$stderr.puts " #{blue(name)}"
definitions.each do |definition|
$stderr.puts " #{yellow(definition.kind.serialize)} #{gray(definition.location.to_s)}"
end
end
$stderr.puts
end

if options[:show_refs]
$stderr.puts "\nReferences:"
index.references.values.flatten.sort_by(&:name).each do |references|
name = references.name
kind = references.kind.serialize
loc = references.location.to_s
$stderr.puts " #{blue(name)} #{yellow(kind)} #{gray(loc)}"
end
$stderr.puts
end

definitions_count = index.definitions.size.to_s
references_count = index.references.size.to_s
$stderr.puts "Analyzing #{blue(definitions_count)} definitions against #{blue(references_count)} references..."

index.finalize!
dead = index.definitions.values.flatten.select(&:dead?)

if options[:sort] == "name"
dead.sort_by!(&:name)
else
dead.sort_by!(&:location)
end

if dead.empty?
$stderr.puts "\n#{green("No dead code found!")}"
else
$stderr.puts "\nCandidates:"
dead.each do |definition|
$stderr.puts " #{red(definition.full_name)} #{gray(definition.location.to_s)}"
end
$stderr.puts "\n"
$stderr.puts red(" Found #{dead.size} dead candidates")

exit(1)
end
end

desc "remove LOCATION", "Remove dead code at LOCATION"
def remove(location_string)
location = Spoom::Deadcode::Location.from_string(location_string)
context = self.context
remover = Spoom::Deadcode::Remover.new(context)

new_source = remover.remove_location(nil, location)
context.write!("PATCH", new_source)

diff = context.exec("diff -u #{location.file} PATCH")
$stderr.puts T.must(diff.out.lines[2..-1]).join
context.remove!("PATCH")

context.write!(location.file, new_source)
rescue Spoom::Deadcode::Remover::Error => e
say_error("Can't remove code at #{location_string}: #{e.message}")
exit(1)
rescue Spoom::Deadcode::Location::LocationError => e
say_error(e.message)
exit(1)
end
end
end
end
28 changes: 23 additions & 5 deletions lib/spoom/deadcode.rb
Expand Up @@ -36,17 +36,35 @@ class IndexerError < Error; end
class << self
extend T::Sig

sig { params(index: Index, ruby: String, file: String, plugins: T::Array[Deadcode::Plugins::Base]).void }
def index_ruby(index, ruby, file:, plugins: [])
node = SyntaxTree.parse(ruby)
visitor = Spoom::Deadcode::Indexer.new(file, ruby, index, plugins: plugins)
visitor.visit(node)
sig { params(ruby: String, file: String).returns(SyntaxTree::Node) }
def parse_ruby(ruby, file:)
SyntaxTree.parse(ruby)
rescue SyntaxTree::Parser::ParseError => e
raise ParserError.new("Error while parsing #{file} (#{e.message} at #{e.lineno}:#{e.column})", parent: e)
end

sig do
params(
index: Index,
node: SyntaxTree::Node,
ruby: String,
file: String,
plugins: T::Array[Deadcode::Plugins::Base],
).void
end
def index_node(index, node, ruby, file:, plugins: [])
visitor = Spoom::Deadcode::Indexer.new(file, ruby, index, plugins: plugins)
visitor.visit(node)
rescue => e
raise IndexerError.new("Error while indexing #{file} (#{e.message})", parent: e)
end

sig { params(index: Index, ruby: String, file: String, plugins: T::Array[Deadcode::Plugins::Base]).void }
def index_ruby(index, ruby, file:, plugins: [])
node = parse_ruby(ruby, file: file)
index_node(index, node, ruby, file: file, plugins: plugins)
end

sig { params(index: Index, erb: String, file: String, plugins: T::Array[Deadcode::Plugins::Base]).void }
def index_erb(index, erb, file:, plugins: [])
ruby = ERB.new(erb).src
Expand Down
11 changes: 11 additions & 0 deletions lib/spoom/deadcode/definition.rb
Expand Up @@ -93,6 +93,17 @@ def ignored?
def ignored!
@status = Status::IGNORED
end

# Utils

sig { params(args: T.untyped).returns(String) }
def to_json(*args)
{
kind: kind,
name: name,
location: location.to_s,
}.to_json
end
end
end
end
18 changes: 12 additions & 6 deletions lib/spoom/deadcode/remover.rb
Expand Up @@ -13,7 +13,7 @@ def initialize(context)
@context = context
end

sig { params(kind: Definition::Kind, location: Location).void }
sig { params(kind: T.nilable(Definition::Kind), location: Location).returns(String) }
def remove_location(kind, location)
file = location.file

Expand All @@ -23,7 +23,7 @@ def remove_location(kind, location)

node_remover = NodeRemover.new(@context.read(file), kind, location)
node_remover.apply_edit
@context.write!(file, node_remover.new_source)
node_remover.new_source
end

class NodeRemover
Expand All @@ -32,7 +32,7 @@ class NodeRemover
sig { returns(String) }
attr_reader :new_source

sig { params(source: String, kind: Definition::Kind, location: Location).void }
sig { params(source: String, kind: T.nilable(Definition::Kind), location: Location).void }
def initialize(source, kind, location)
@old_source = source
@new_source = T.let(source.dup, String)
Expand Down Expand Up @@ -296,7 +296,13 @@ def replace_chars(start_char, end_char, replacement)
@new_source[start_char...end_char] = replacement
end

sig { params(node: SyntaxTree::MethodAddBlock, name: String, kind: Definition::Kind).returns(String) }
sig do
params(
node: SyntaxTree::MethodAddBlock,
name: String,
kind: T.nilable(Definition::Kind),
).returns(String)
end
def transform_sig(node, name:, kind:)
type = T.let(nil, T.nilable(String))

Expand Down Expand Up @@ -493,7 +499,7 @@ class NodeFinder < SyntaxTree::Visitor
class << self
extend T::Sig

sig { params(source: String, location: Location, kind: Definition::Kind).returns(NodeContext) }
sig { params(source: String, location: Location, kind: T.nilable(Definition::Kind)).returns(NodeContext) }
def find(source, location, kind)
tree = SyntaxTree.parse(source)

Expand All @@ -505,7 +511,7 @@ def find(source, location, kind)
raise Error, "Can't find node at #{location}"
end

unless node_match_kind?(node, kind)
if kind && !node_match_kind?(node, kind)
raise Error, "Can't find node at #{location}, expected #{kind} but got #{node.class}"
end

Expand Down
1 change: 1 addition & 0 deletions test/spoom/cli/cli_test.rb
Expand Up @@ -27,6 +27,7 @@ def test_display_help_long_option
spoom --version # Show version
spoom bump # Bump Sorbet sigils from `false` to `true` when no errors
spoom coverage # Collect metrics related to Sorbet coverage
spoom deadcode # Analyze code to find deadcode
spoom help [COMMAND] # Describe available commands or one specific command
spoom lsp # Send LSP requests to Sorbet
spoom tc # Run Sorbet and parses its output
Expand Down