From 7578effd321dbe07dbaeb2b88ec2b7f93400cbda Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Thu, 6 Aug 2026 12:03:05 +0100 Subject: [PATCH] Add an in-project linter rule and rule test support Assisted-By: devx/348380ff-5362-4e75-b229-8f6db00a32f6 --- .github/workflows/ci.yml | 4 + lib/rubydex/cli/command/lint.rb | 5 +- lib/rubydex/graph.rb | 3 +- lib/rubydex/linter/rule_loader.rb | 13 +- lib/rubydex/linter/rule_test_case.rb | 343 ++++++++++++++++++ lib/rubydex/linter/runner.rb | 3 +- lib/rubydex_linter/rules/rule_structure.rb | 125 +++++++ rbi/rubydex.rbi | 69 ++++ test/cli_test.rb | 20 +- test/graph_test.rb | 21 ++ test/linter_test.rb | 14 + .../rules/rule_structure_test.rb | 124 +++++++ test/rule_loader_test.rb | 10 + 13 files changed, 736 insertions(+), 18 deletions(-) create mode 100644 lib/rubydex/linter/rule_test_case.rb create mode 100644 lib/rubydex_linter/rules/rule_structure.rb create mode 100644 test/rubydex_linter/rules/rule_structure_test.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5fd0c11b5..2e5926a82 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,6 +67,10 @@ jobs: - name: Run Ruby tests run: bundle exec rake test + - name: Run Rubydex linter + if: matrix.os == 'ubuntu-latest' && matrix.ruby == '4.0' + run: bundle exec rdx lint . + - name: Save Rust compile cache id: rust-compile-cache-save uses: actions/cache/save@v6 diff --git a/lib/rubydex/cli/command/lint.rb b/lib/rubydex/cli/command/lint.rb index 80f86c570..15074b037 100644 --- a/lib/rubydex/cli/command/lint.rb +++ b/lib/rubydex/cli/command/lint.rb @@ -29,7 +29,10 @@ def run warn_unknown_rules(config.linter, rules) graph = build_graph($stderr, workspace_path:, config:, fail_on_index_errors: true) - result = Rubydex::Linter::Runner.new(graph, rules:, config: config.linter).run + runner = Rubydex::Linter::Runner.new(graph, rules:, config: config.linter) + rule_count = runner.rules.size + $stderr.puts("Running #{rule_count} #{pluralize("rule", rule_count)}...") + result = runner.run if result.diagnostics.empty? print_summary(graph.documents.count, result.diagnostics) return diff --git a/lib/rubydex/graph.rb b/lib/rubydex/graph.rb index cc8f7d62c..d807c9132 100644 --- a/lib/rubydex/graph.rb +++ b/lib/rubydex/graph.rb @@ -62,7 +62,8 @@ def add_workspace_dependency_paths(paths) # descending them next if File.absolute_path?(path) - paths << File.join(spec.full_gem_path, path) + require_path = File.join(spec.full_gem_path, path) + paths << require_path if File.directory?(require_path) end rescue Gem::MissingSpecError nil diff --git a/lib/rubydex/linter/rule_loader.rb b/lib/rubydex/linter/rule_loader.rb index 80788a5c9..46d49bf91 100644 --- a/lib/rubydex/linter/rule_loader.rb +++ b/lib/rubydex/linter/rule_loader.rb @@ -5,17 +5,17 @@ module Linter # Loads project and bundled-gem rules using the Rubydex linter path convention. class RuleLoader RULE_GLOB = "rubydex_linter/rules/**/*.rb" #: String + BUILT_IN_RULE_GLOB = File.expand_path("../../rubydex_linter/rules/**/*.rb", __dir__) #: String class << self #: (String workspace_path) -> Array[singleton(Rule)] def load(workspace_path) existing_rules = Rule.subclasses - rule_files = Dir.glob(RULE_GLOB, base: workspace_path).map do |rule_file| + rule_files = Dir.glob(BUILT_IN_RULE_GLOB) + rule_files.concat(Dir.glob(RULE_GLOB, base: workspace_path).map do |rule_file| File.expand_path(rule_file, workspace_path) - end - if ENV["BUNDLE_GEMFILE"] - rule_files.concat(Gem.find_latest_files(RULE_GLOB)) - end + end) + rule_files.concat(Gem.find_latest_files(RULE_GLOB)) if ENV["BUNDLE_GEMFILE"] rule_files.each do |rule_file| require rule_file @@ -23,7 +23,8 @@ def load(workspace_path) raise RuleLoadError, "Unable to load linter rules from #{rule_file}: #{error.message}", cause: error end - Rule.subclasses - existing_rules + built_in_rules = [Rules::RuleStructure] #: Array[singleton(Rule)] + built_in_rules | (Rule.subclasses - existing_rules) end end end diff --git a/lib/rubydex/linter/rule_test_case.rb b/lib/rubydex/linter/rule_test_case.rb new file mode 100644 index 000000000..a2dff1516 --- /dev/null +++ b/lib/rubydex/linter/rule_test_case.rb @@ -0,0 +1,343 @@ +# frozen_string_literal: true + +require "fileutils" +require "minitest/test" +require "pathname" +require "tmpdir" +require "uri" +require "rubydex/linter" + +module Rubydex + module Linter + # Base test case for Rubydex linter rules. + # + # Add expected diagnostics below the source: + # + # assert_diagnostics(<<~RUBY) + # FOO = 123 + # ^^^ Failure: FOO + # RUBY + # + # The carets mark the diagnostic range. Use `^{}` for a zero-width range. + # Pass a hash to test multiple files. Repeat an annotation line for each message line. + # By default, `FooTest` tests the `Foo` rule. + class RuleTestCase < Minitest::Test + DEFAULT_FILE = "test.rb" #: String + + ANNOTATION_PATTERN = /\A(?\s*)(?(?.*))?\z/ #: Regexp + + #: type annotation = [Integer, Integer, Integer, String] + + #: String + attr_reader :workspace_path + + #: (String) -> void + def initialize(name) + super + @workspace_path = File.realpath(Dir.mktmpdir("rubydex-rule-test-")) #: String + end + + #: -> void + def teardown + super + ensure + FileUtils.remove_entry(workspace_path) + end + + #: -> singleton(Rule) + def rule_class + @rule_class ||= begin + name = self.class.to_s.delete_suffix("Test") + if name == self.class.to_s + raise "Could not infer rule class from #{self.class}; override `#rule_class`" + end + + Object.const_get(name) #: as singleton(Rule) + end #: singleton(Rule)? + end + + # Registers sources that are loaded into the graph for every assertion. + # Shared sources must not contain caret annotations. Test-specific + # sources win on filename collision. + #: (Hash[String, String]) -> void + def add_shared_source(sources) + shared_sources.merge!(sources) + end + + # Returns absolute file paths whose diagnostics should be ignored. + #: -> Array[String] + def ignored_diagnostic_files + @ignored_diagnostic_files ||= [] #: Array[String]? + end + + #: -> LinterConfig + def rule_config + @rule_config ||= LinterConfig.new({}) #: LinterConfig? + end + + # Runs the rule and compares every diagnostic location with the inline + # annotations in the corresponding source. + #: (*(String | Hash[String | Symbol, String])) ?{ (Graph) -> Rule } -> Array[Diagnostic] + def assert_diagnostics(*args, &rule_builder) + sources = validated_shared_source.merge(normalize_sources(args)) + clean_per_file, expected_per_file = write_sources(sources) + + diagnostics = run_rule(&rule_builder) + actual_per_file = annotations_for_diagnostics(diagnostics, clean_per_file) + + surprise_files = actual_per_file.keys - clean_per_file.keys - ignored_diagnostic_files + refute_predicate( + surprise_files, + :any?, + "Diagnostics in unexpected files: #{surprise_files.inspect}", + ) + + clean_per_file.each do |filename, clean| + expected = render_annotated(clean, expected_per_file[filename] || []) + actual = render_annotated(clean, actual_per_file[filename] || []) + assert_equal(expected, actual, "Mismatch in #{filename}") + end + + diagnostics + end + + # Runs the rule and asserts no diagnostics are reported in the provided + # sources. Sources must not contain caret annotations. + #: (*(String | Hash[String | Symbol, String])) ?{ (Graph) -> Rule } -> Array[Diagnostic] + def assert_no_diagnostics(*args, &rule_builder) + sources = validated_shared_source.merge(normalize_sources(args)) + write_sources(sources) + + diagnostics = run_rule(&rule_builder) + ignored = ignored_diagnostic_files + reported = diagnostics.reject { |diagnostic| ignored.include?(diagnostic.location.to_file_path) } + assert_empty(reported, "Expected no diagnostics, got #{reported.map(&:message).inspect}") + diagnostics + end + + #: ( + #| String, + #| *(String | Hash[String | Symbol, String]), + #| ?after_excluding: Array[String], + #| ) ?{ (Graph) -> Rule } -> void + def assert_handles_missing_required_dependency(dependency, *args, after_excluding: [], &rule_builder) + sources = validated_shared_source.dup #: Hash[String, String] + after_excluding.each { |filename| sources.delete(filename) } + sources.merge!(normalize_sources(args)) + write_sources(sources) + + error = assert_raises(MissingGraphDependencyError) do + run_rule(&rule_builder) + end + + expected_message = MissingGraphDependencyError.new(rule_class.rule_name, dependency).message + assert_equal(expected_message, error.message) + end + + private + + #: (Hash[String, String]) -> [Hash[String, String], Hash[String, Array[annotation]]] + def write_sources(sources) + clean_per_file = {} #: Hash[String, String] + expected_per_file = {} #: Hash[String, Array[annotation]] + + virtual_sources.clear + + sources.each do |filename, annotated| + clean, annotations = parse_annotations(annotated) + if Pathname(filename).absolute? + absolute_path = filename + virtual_sources[absolute_path] = clean + else + absolute_path = File.join(workspace_path, filename) + FileUtils.mkdir_p(File.dirname(absolute_path)) + File.write(absolute_path, clean) + absolute_path = File.realpath(absolute_path) + end + clean_per_file[absolute_path] = clean + expected_per_file[absolute_path] = annotations + end + + [clean_per_file, expected_per_file] + end + + #: -> Hash[String, String] + def shared_sources + @shared_sources ||= {} #: Hash[String, String]? + end + + #: -> Hash[String, String] + def virtual_sources + @virtual_sources ||= {} #: Hash[String, String]? + end + + #: -> Hash[String, String] + def validated_shared_source + shared_sources.each do |filename, source| + _, annotations = parse_annotations(source) + raise "Shared source #{filename} must not contain caret annotations" unless annotations.empty? + end + shared_sources + end + + #: (Array[String | Hash[String | Symbol, String]]) -> Hash[String, String] + def normalize_sources(args) + case args + in [] + {} + in [String => source] + { DEFAULT_FILE => source } + in [Hash => hash] + hash.transform_keys(&:to_s) + else + raise ArgumentError, "expected a String or a { filename => source } Hash, got: #{args.inspect}" + end + end + + #: (String) -> [String, Array[annotation]] + def parse_annotations(annotated_source) + clean = [] #: Array[String] + annotations = [] #: Array[annotation] + + annotated_source.each_line do |line| + if (match = ANNOTATION_PATTERN.match(line.chomp)) + indent = match[:indent]&.length #: as !nil + carets = match[:carets] #: as !nil + start_column = indent + end_column = if carets == "^{}" + indent + else + indent + carets.length + end + + target_line = clean.empty? ? 0 : clean.size - 1 + message = match[:message].to_s + last = annotations.last + if last && last[0] == target_line && last[1] == start_column && last[2] == end_column + last[3] = "#{last[3]}\n#{message}" + else + annotations << [target_line, start_column, end_column, message] + end + else + clean << line + end + end + + [clean.join, annotations] + end + + #: ?{ (Graph) -> Rule } -> Array[Diagnostic] + def run_rule(&rule_builder) + graph = Graph.configure_for_workspace(workspace_path) + file_paths = workspace_file_paths + graph.index_all(file_paths.reject { |path| File.extname(path) == ".rake" }) + index_rake_files(graph, file_paths) + + virtual_sources.each do |path, source| + graph.index_source(uri_for_path(path), source, source_id_for(path)) + end + graph.resolve + + rule = rule_builder ? rule_builder.call(graph) : rule_class.new(graph, config: rule_config) + rule.lint + rule.diagnostics + end + + #: (Graph, Array[String]) -> void + def index_rake_files(graph, file_paths) + file_paths.each do |path| + next unless File.extname(path) == ".rake" + + graph.index_source(uri_for_path(path), File.read(path), "ruby") + end + end + + #: -> Array[String] + def workspace_file_paths + Dir.glob("**/*", base: workspace_path).sort.filter_map do |relative_path| + absolute_path = File.join(workspace_path, relative_path) + next unless File.file?(absolute_path) + + case File.extname(relative_path) + when ".rb", ".rbi", ".rake", ".rbs" + File.realpath(absolute_path) + else + raise "Unsupported file type: #{relative_path}" + end + end + end + + #: (String) -> String + def source_id_for(path) + case File.extname(path) + when ".rb", ".rbi", ".rake", ".ru" + "ruby" + when ".rbs" + "rbs" + else + raise "Unsupported file type: #{path}" + end + end + + #: (Array[Diagnostic], Hash[String, String]) -> Hash[String, Array[annotation]] + def annotations_for_diagnostics(diagnostics, sources) + result = Hash.new { |hash, key| hash[key] = [] } #: Hash[String, Array[annotation]] + + diagnostics.each do |diagnostic| + located_messages = [[diagnostic.location, diagnostic.message]] #: Array[[Location, String]] + diagnostic.related_information.each do |related| + located_messages << [related.location, related.message] + end + + located_messages.each do |location, message| + path = location.to_file_path + line_index = location.start_line + start_column = location.start_column + end_column = + if location.end_line == location.start_line + location.end_column + else + clean = sources[path] + line_text = clean ? (clean.each_line.to_a[line_index] || "").chomp : "" + line_text.length + end + + annotations = result[path] #: as !nil + annotations << [line_index, start_column, end_column, message.chomp] + end + end + + result + end + + #: (String, Array[annotation]) -> String + def render_annotated(clean_source, annotations) + lines = clean_source.each_line.to_a + sorted = annotations.sort_by do |line_index, start_column, end_column, message| + [line_index, start_column, end_column, message] + end + output = lines.dup + + sorted.reverse_each do |line_index, start_column, end_column, message| + caret_count = end_column - start_column + indentation = " " * start_column + carets = caret_count.zero? ? "^{}" : "^" * caret_count + markers = if message.empty? + ["#{indentation}#{carets}\n"] + else + message.split("\n", -1).map { |line| "#{indentation}#{carets} #{line}\n" } + end + output[line_index + 1, 0] = markers + end + + output.join + end + + #: (String) -> String + def uri_for_path(path) + uri_path = Gem.win_platform? ? "/#{path}" : path + URI::File.build(path: uri_path).to_s + end + end + end +end diff --git a/lib/rubydex/linter/runner.rb b/lib/rubydex/linter/runner.rb index 5c8c054df..305ee8ead 100644 --- a/lib/rubydex/linter/runner.rb +++ b/lib/rubydex/linter/runner.rb @@ -29,7 +29,8 @@ def run filter_diagnostics(rule.diagnostics, @config.excludes_for(rule_class)) end - diagnostics = (@graph.diagnostics + rule_diagnostics).select do |diagnostic| + graph_diagnostics = filter_diagnostics(@graph.diagnostics, []) + diagnostics = (graph_diagnostics + rule_diagnostics).select do |diagnostic| diagnostic_in_workspace?(diagnostic) end.sort_by do |diagnostic| location = diagnostic.location diff --git a/lib/rubydex_linter/rules/rule_structure.rb b/lib/rubydex_linter/rules/rule_structure.rb new file mode 100644 index 000000000..8e8adc7b4 --- /dev/null +++ b/lib/rubydex_linter/rules/rule_structure.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +require "rubydex/linter" + +module Rubydex + module Linter + module Rules + # Ensures discovered linter rule files contain one correctly placed and named rule class. + class RuleStructure < Rule + include Helpers::SourceAccessHelpers + + BASE_RULE_NAME = "Rubydex::Linter::Rule" #: String + RULE_NAMESPACE = "Rubydex::Linter::Rules" #: String + RULE_FILE_PATTERNS = [ + "rubydex_linter/rules/**/*.rb", + "lib/rubydex_linter/rules/**/*.rb", + ].freeze #: Array[String] + TEST_FILE_PATTERNS = ["test/**/*", "**/test/**/*"].freeze #: Array[String] + + # @override + #: -> singleton(Severity::Base) + def severity + Severity::Error + end + + # @override + #: -> void + def lint + rule_class_names = child_classes(BASE_RULE_NAME).to_h { |rule_class| [rule_class.name, true] } + + graph.documents.each do |document| + path = path_for_uri(document.uri) + next unless workspace_path?(path) + + rule_file = rule_file?(path) + next if !rule_file && test_file?(path) + + definitions = rule_definitions(document, rule_class_names) + validate_rule_file(document, definitions) if rule_file + + definitions.each do |name, definition| + validate_rule_definition(name, definition, path) + end + end + end + + private + + #: (Document, Hash[String, bool]) -> Hash[String, ClassDefinition] + def rule_definitions(document, rule_class_names) + definitions = {} #: Hash[String, ClassDefinition] + + document.definitions.each do |definition| + next unless definition.is_a?(ClassDefinition) + + declaration = definition.declaration + next unless declaration + next unless rule_class_names.key?(declaration.name) + + definitions[declaration.name] ||= definition + end + + definitions + end + + #: (Document, Hash[String, ClassDefinition]) -> void + def validate_rule_file(document, definitions) + return if definitions.one? + + add_diagnostic( + "Each rule file must define exactly one class that inherits from `#{BASE_RULE_NAME}`; " \ + "found #{definitions.length}.", + file_location(document.uri), + related_information: definitions.map do |name, definition| + RelatedInformation.new("`#{name}` is defined here.", diagnostic_location(definition)) + end, + ) + end + + #: (String, ClassDefinition, String) -> void + def validate_rule_definition(name, definition, path) + unless rule_file?(path) + add_diagnostic( + "`#{name}` must be defined under `rubydex_linter/rules/` or `lib/rubydex_linter/rules/`.", + diagnostic_location(definition), + ) + end + + return if name.start_with?("#{RULE_NAMESPACE}::") + + add_diagnostic( + "`#{name}` must be defined under `#{RULE_NAMESPACE}`.", + diagnostic_location(definition), + ) + end + + #: (String) -> bool + def workspace_path?(path) + workspace = graph.workspace_path + path == workspace || path.start_with?("#{workspace}/") + end + + #: (String) -> bool + def rule_file?(path) + path_matches_patterns?(path, RULE_FILE_PATTERNS) + end + + #: (String) -> bool + def test_file?(path) + path_matches_patterns?(path, TEST_FILE_PATTERNS) + end + + #: (String, Array[String]) -> bool + def path_matches_patterns?(path, patterns) + Helpers::PathHelpers.path_matches_patterns?( + path, + patterns, + workspace: graph.workspace_path, + flags: Helpers::PathHelpers::RUBOCOP_EXCLUDE_FNMATCH_FLAGS, + ) + end + end + end + end +end diff --git a/rbi/rubydex.rbi b/rbi/rubydex.rbi index d812272c4..fe3fc586f 100644 --- a/rbi/rubydex.rbi +++ b/rbi/rubydex.rbi @@ -358,6 +358,7 @@ end module Rubydex::Linter; end module Rubydex::Linter::Helpers; end +module Rubydex::Linter::Rules; end module Rubydex::Linter::Helpers::PathHelpers extend T::Helpers @@ -491,11 +492,79 @@ class Rubydex::Linter::RuleLoadError < StandardError; end class Rubydex::Linter::RuleLoader RULE_GLOB = T.let(T.unsafe(nil), String) + BUILT_IN_RULE_GLOB = T.let(T.unsafe(nil), String) sig { params(workspace_path: String).returns(T::Array[T.class_of(Rubydex::Linter::Rule)]) } def self.load(workspace_path); end end +class Rubydex::Linter::Rules::RuleStructure < Rubydex::Linter::Rule + include Rubydex::Linter::Helpers::SourceAccessHelpers + + BASE_RULE_NAME = T.let(T.unsafe(nil), String) + RULE_NAMESPACE = T.let(T.unsafe(nil), String) + RULE_FILE_PATTERNS = T.let(T.unsafe(nil), T::Array[String]) + TEST_FILE_PATTERNS = T.let(T.unsafe(nil), T::Array[String]) + + sig { returns(T.class_of(Rubydex::Severity::Base)) } + def severity; end + + sig { void } + def lint; end +end + +class Rubydex::Linter::RuleTestCase < ::Minitest::Test + DEFAULT_FILE = T.let(T.unsafe(nil), String) + ANNOTATION_PATTERN = T.let(T.unsafe(nil), Regexp) + + sig { returns(String) } + def workspace_path; end + + sig { params(name: String).void } + def initialize(name); end + + sig { void } + def teardown; end + + sig { returns(T.class_of(Rubydex::Linter::Rule)) } + def rule_class; end + + sig { params(sources: T::Hash[String, String]).void } + def add_shared_source(sources); end + + sig { returns(T::Array[String]) } + def ignored_diagnostic_files; end + + sig { returns(Rubydex::LinterConfig) } + def rule_config; end + + sig do + params( + args: T.any(String, T::Hash[T.any(String, Symbol), String]), + rule_builder: T.nilable(T.proc.params(graph: Rubydex::Graph).returns(Rubydex::Linter::Rule)), + ).returns(T::Array[Rubydex::Diagnostic]) + end + def assert_diagnostics(*args, &rule_builder); end + + sig do + params( + args: T.any(String, T::Hash[T.any(String, Symbol), String]), + rule_builder: T.nilable(T.proc.params(graph: Rubydex::Graph).returns(Rubydex::Linter::Rule)), + ).returns(T::Array[Rubydex::Diagnostic]) + end + def assert_no_diagnostics(*args, &rule_builder); end + + sig do + params( + dependency: String, + args: T.any(String, T::Hash[T.any(String, Symbol), String]), + after_excluding: T::Array[String], + rule_builder: T.nilable(T.proc.params(graph: Rubydex::Graph).returns(Rubydex::Linter::Rule)), + ).void + end + def assert_handles_missing_required_dependency(dependency, *args, after_excluding: [], &rule_builder); end +end + class Rubydex::Linter::Runner sig do params( diff --git a/test/cli_test.rb b/test/cli_test.rb index 60cfb2f53..547a4a249 100644 --- a/test/cli_test.rb +++ b/test/cli_test.rb @@ -281,6 +281,7 @@ class Foo; end refute_stdout_includes(result, context.absolute_path) assert_stderr_includes(result, "Indexing workspace...") assert_stderr_includes(result, "Resolving graph...") + assert_stderr_includes(result, "Running 2 rules...") end end @@ -300,13 +301,13 @@ def test_lint_loads_rules_from_bundled_dependencies with_context do |context| rule_path = "fake_gem/lib/rubydex_linter/rules/no_foo.rb" write_linter_rule(context, "CLITestDependencyErrorRule", path: rule_path) - context.write!("app.rb", "class Foo; end\n") + context.write!("workspace/app.rb", "class Foo; end\n") Gem.expects(:find_latest_files) .with("rubydex_linter/rules/**/*.rb") .returns([context.absolute_path_to(rule_path)]) result = with_bundle_gemfile(context.absolute_path_to("Gemfile")) do - rdx("lint", context.absolute_path) + rdx("lint", context.absolute_path_to("workspace")) end refute_success_status(result) @@ -314,16 +315,17 @@ def test_lint_loads_rules_from_bundled_dependencies end end - def test_lint_requires_a_discovered_rule_before_indexing + def test_lint_runs_built_in_rules_without_project_rules with_context do |context| - context.write!("app.rb", "class Foo; end\n") + context.write!("app.rb", "class Bar; end\n") result = with_bundle_gemfile(nil) { rdx("lint", context.absolute_path) } - refute_success_status(result) - assert_empty_stdout(result) - assert_stderr_includes(result, "No Rubydex::Linter::Rule subclasses were loaded") - refute_stderr_includes(result, "Indexing workspace...") + assert_success_status(result) + assert_stdout_includes_pattern(result, /\d+ files inspected, no offenses detected/) + assert_stderr_includes(result, "Indexing workspace...") + assert_stderr_includes(result, "Running 1 rule...") + refute_stderr_includes(result, "No Rubydex::Linter::Rule subclasses were loaded") end end @@ -406,7 +408,7 @@ def write_linter_rule(context, class_name, path: "rubydex_linter/rules/no_foo.rb context.write!(path, <<~RUBY) # frozen_string_literal: true - class #{class_name} < Rubydex::Linter::Rule + class Rubydex::Linter::Rules::#{class_name} < Rubydex::Linter::Rule def severity = Rubydex::Severity::Error def lint diff --git a/test/graph_test.rb b/test/graph_test.rb index 36a69cee0..28c662440 100644 --- a/test/graph_test.rb +++ b/test/graph_test.rb @@ -3,6 +3,7 @@ require "test_helper" require "helpers/context" require "json" +require "mocha/minitest" class GraphTest < Minitest::Test include Test::Helpers::WithContext @@ -764,6 +765,26 @@ def test_workspace_paths end end + def test_index_workspace_skips_missing_dependency_require_paths + with_context do |context| + context.write!("workspace/app.rb", "class App; end\n") + context.write!("dependency/lib/dependency.rb", "class WorkspacePathsDependency; end\n") + Bundler.stubs(:locked_gems).returns(stub(specs: [stub(name: "dependency")])) + Gem::Specification.expects(:find_by_name).with("dependency").returns( + stub( + full_gem_path: context.absolute_path_to("dependency"), + require_paths: ["lib", "missing"], + ), + ) + graph = Rubydex::Graph.configure_for_workspace(context.absolute_path_to("workspace")) + + assert_empty(graph.index_workspace) + graph.resolve + + refute_nil(graph["WorkspacePathsDependency"]) + end + end + def test_index_workspace_includes_rbs_core_definitions graph = Rubydex::Graph.new graph.index_workspace diff --git a/test/linter_test.rb b/test/linter_test.rb index a5784d796..55e253208 100644 --- a/test/linter_test.rb +++ b/test/linter_test.rb @@ -221,6 +221,20 @@ def test_runner_filters_diagnostics_under_dependency_paths end end + def test_runner_filters_native_diagnostics_under_dependency_paths + with_context do |context| + context.write!("workspace/inside.rb") + graph = Rubydex::Graph.configure_for_workspace(context.absolute_path_to("workspace")) + dependency_path = context.absolute_path_to("workspace/.dev/gem") + graph.index_source(context.uri_to("workspace/.dev/gem/broken.rb"), "class Broken", "ruby") + Gem.stubs(:path).returns([dependency_path]) + + result = Rubydex::Linter::Runner.new(graph, rules: [SilentRule], config: linter_config).run + + assert_empty(result.diagnostics) + end + end + def test_runner_filters_a_diagnostic_when_its_primary_location_matches_a_rule_exclude with_context do |context| context.write!("workspace/inside.rb") diff --git a/test/rubydex_linter/rules/rule_structure_test.rb b/test/rubydex_linter/rules/rule_structure_test.rb new file mode 100644 index 000000000..fab40fb43 --- /dev/null +++ b/test/rubydex_linter/rules/rule_structure_test.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require "test_helper" +require "rubydex/linter/rule_test_case" +require "rubydex_linter/rules/rule_structure" + +module Rubydex + module Linter + module Rules + class RuleStructureTest < RuleTestCase + BASE_RULE_SOURCE = <<~RUBY + module Rubydex + module Linter + class Rule; end + module Rules; end + end + end + RUBY + + def setup + super + add_shared_source("lib/rubydex/linter/rule.rb" => BASE_RULE_SOURCE) + end + + def test_allows_one_rule_class_in_each_supported_rule_directory + assert_no_diagnostics( + "rubydex_linter/rules/project_rule.rb" => rule_source("ProjectRule"), + "lib/rubydex_linter/rules/gem_rule.rb" => rule_source("GemRule"), + ) + end + + def test_allows_indirect_rule_subclasses_and_helper_classes + assert_no_diagnostics( + "rubydex_linter/rules/base_rule.rb" => rule_source("BaseRule"), + "rubydex_linter/rules/indirect_rule.rb" => <<~RUBY, + class Rubydex::Linter::Rules::Helper; end + class Rubydex::Linter::Rules::IndirectRule < Rubydex::Linter::Rules::BaseRule; end + RUBY + ) + end + + def test_reports_a_rule_file_without_a_rule_class + diagnostics = assert_diagnostics( + "rubydex_linter/rules/not_a_rule.rb" => <<~RUBY, + class Rubydex::Linter::Rules::NotARule; end + ^{} Each rule file must define exactly one class that inherits from `Rubydex::Linter::Rule`; found 0. + RUBY + ) + + diagnostic = diagnostics.fetch(0) + assert_same(Severity::Error, diagnostic.severity) + assert_equal("RuleStructure", diagnostic.rule) + end + + def test_reports_multiple_rule_classes_in_one_file + diagnostics = assert_diagnostics( + "rubydex_linter/rules/two_rules.rb" => <<~RUBY, + class Rubydex::Linter::Rules::FirstRule < Rubydex::Linter::Rule; end + ^{} Each rule file must define exactly one class that inherits from `Rubydex::Linter::Rule`; found 2. + ^^^^^^^^^ `Rubydex::Linter::Rules::FirstRule` is defined here. + class Rubydex::Linter::Rules::SecondRule < Rubydex::Linter::Rule; end + ^^^^^^^^^^ `Rubydex::Linter::Rules::SecondRule` is defined here. + RUBY + ) + + assert_equal(1, diagnostics.length) + assert_equal( + [ + "`Rubydex::Linter::Rules::FirstRule` is defined here.", + "`Rubydex::Linter::Rules::SecondRule` is defined here.", + ], + diagnostics.fetch(0).related_information.map(&:message), + ) + end + + def test_reports_a_rule_class_outside_the_rules_namespace + assert_diagnostics( + "rubydex_linter/rules/wrong_namespace.rb" => <<~RUBY, + module ConsumerRules + class WrongNamespace < Rubydex::Linter::Rule; end + ^^^^^^^^^^^^^^ `ConsumerRules::WrongNamespace` must be defined under `Rubydex::Linter::Rules`. + end + RUBY + ) + end + + def test_reports_a_rule_class_outside_a_rule_directory + assert_diagnostics( + "app/rules/wrong_place.rb" => <<~RUBY, + class Rubydex::Linter::Rules::WrongPlace < Rubydex::Linter::Rule; end + ^^^^^^^^^^ `Rubydex::Linter::Rules::WrongPlace` must be defined under `rubydex_linter/rules/` or `lib/rubydex_linter/rules/`. + RUBY + ) + end + + def test_ignores_rule_fixture_classes_under_test_directories + assert_no_diagnostics( + "test/fixtures/fixture_rule.rb" => <<~RUBY, + module RuleStructureTestFixtures + class FixtureRule < Rubydex::Linter::Rule; end + end + RUBY + ) + end + + def test_checks_rule_files_nested_under_a_test_directory + assert_diagnostics( + "rubydex_linter/rules/test/not_a_rule.rb" => <<~RUBY, + class NotARule; end + ^{} Each rule file must define exactly one class that inherits from `Rubydex::Linter::Rule`; found 0. + RUBY + ) + end + + private + + #: (String) -> String + def rule_source(class_name) + "class Rubydex::Linter::Rules::#{class_name} < Rubydex::Linter::Rule; end\n" + end + end + end + end +end diff --git a/test/rule_loader_test.rb b/test/rule_loader_test.rb index c7ec83cc5..3f6238387 100644 --- a/test/rule_loader_test.rb +++ b/test/rule_loader_test.rb @@ -7,6 +7,16 @@ class RuleLoaderTest < Minitest::Test include Test::Helpers::WithContext + def test_load_returns_built_in_rules_without_bundler + with_context do |context| + rules = with_bundle_gemfile(nil) do + Rubydex::Linter::RuleLoader.load(context.absolute_path) + end + + assert_includes(rules, Rubydex::Linter::Rules::RuleStructure) + end + end + def test_load_wraps_rule_file_errors with_context do |context| rule_file = "rubydex_linter/rules/broken_rule.rb"