Initial proof of concept for expanding ameba's functionality with semantic information

spec/ameba/rule/lint/unknown_type_spec.cr

@@ -0,0 +1,28 @@
+require "../../../spec_helper"
+
+module Ameba::Rule::Lint
+  subject = UnknownType.new
+
+  describe UnknownType do
+    it "passes if types are known" do
+      expect_no_issues subject, <<-CRYSTAL, semantic: true
+        a : Int32 = 1
+
+        def hello(name : String)
+          puts "Hello, \#{name}!"
+        end
+        CRYSTAL
+    end
+
+    it "fails if types are unknown" do
+      expect_issue subject, <<-CRYSTAL, semantic: true
+        count : Int3 = 1
+              # ^^^^ error: Unknown type
+        def hello(name : Str)
+                       # ^^^ error: Unknown type
+          puts "Hello, #{name}!"
+        end
+        CRYSTAL
+    end
+  end
+end

spec/ameba/runner_spec.cr

@@ -54,13 +54,13 @@ module Ameba
         source = Source.new "", "source.cr"
 
         v1_0_0 = SemanticVersion.parse("1.0.0")
-        Runner.new(rules, [source], formatter, default_severity, false, v1_0_0).run.success?.should be_true
+        Runner.new(rules, [source], formatter, default_severity, false, false, v1_0_0).run.success?.should be_true
 
         v1_5_0 = SemanticVersion.parse("1.5.0")
-        Runner.new(rules, [source], formatter, default_severity, false, v1_5_0).run.success?.should be_false
+        Runner.new(rules, [source], formatter, default_severity, false, false, v1_5_0).run.success?.should be_false
 
         v1_10_0 = SemanticVersion.parse("1.10.0")
-        Runner.new(rules, [source], formatter, default_severity, false, v1_10_0).run.success?.should be_false
+        Runner.new(rules, [source], formatter, default_severity, false, false, v1_10_0).run.success?.should be_false
       end
 
       it "skips rule check if source is excluded" do

src/ameba/ast/visitors/semantic_visitor.cr

@@ -0,0 +1,63 @@
+module Ameba::AST
+  # A simple visitor that iterates over a single files parser-only tree, resolving the
+  # namespaces to their corresponding namespace types.
+  #
+  # TODO: expand this to also include information about the current scope
+  class SemanticVisitor < BaseVisitor
+    getter context : SemanticContext
+    getter current_type : Crystal::ModuleType
+
+    def initialize(@rule, @source, @context : SemanticContext)
+      @current_type = @context.program
+
+      super(@rule, @source)
+    end
+
+    def visit(node : Crystal::ClassDef | Crystal::ModuleDef | Crystal::LibDef)
+      @rule.test(@source, node, current_type)
+
+      # `Lint/NoSemanticInformation` should catch this edge case
+      type = current_type.lookup_type?(node.name).try(&.as?(Crystal::ModuleType))
+
+      # Don't visit bodies of types we don't have any semantic information for.
+      # This is reported by `Lint/NoSemanticInformation`
+      return false unless type
+
+      pushing_type(type) do
+        node.body.accept(self)
+      end
+
+      false
+    end
+
+    def visit(node : Crystal::EnumDef)
+      @rule.test(@source, node, current_type)
+
+      # `Lint/NoSemanticInformation` should catch this edge case
+      type = current_type.lookup_type?(node.name).try(&.as?(Crystal::ModuleType))
+
+      # Don't visit bodies of types we don't have any semantic information for.
+      # This is reported by `Lint/NoSemanticInformation`
+      return false unless type
+
+      pushing_type(type) do
+        node.members.each &.accept(self)
+      end
+
+      false
+    end
+
+    def visit(node) : Bool
+      @rule.test(@source, node, current_type)
+
+      true
+    end
+
+    private def pushing_type(type : Crystal::ModuleType, &)
+      old_type = @current_type
+      @current_type = type
+      yield
+      @current_type = old_type
+    end
+  end
+end

src/ameba/cli/cmd.cr

@@ -23,6 +23,7 @@ module Ameba::Cli
     property? colors = true
     property? without_affected_code = false
     property? autocorrect = false
+    property? semantic = false
   end
 
   def run(args = ARGV) : Nil
@@ -151,6 +152,10 @@ module Ameba::Cli
       parser.on("--stdin-filename FILENAME", "Read source from STDIN") do |file|
         opts.stdin_filename = file
       end
+
+      parser.on("--semantic", "Semantic analysis") do
+        opts.semantic = true
+      end
     end
 
     opts
@@ -170,6 +175,9 @@ module Ameba::Cli
     if fail_level = opts.fail_level
       config.severity = fail_level
     end
+    if semantic = opts.semantic?
+      config.semantic = semantic
+    end
 
     configure_formatter(config, opts)
     configure_rules(config, opts)

src/ameba/config.cr

@@ -96,6 +96,10 @@ class Ameba::Config
   # Returns `true` if correctable issues should be autocorrected.
   property? autocorrect = false
 
+  # Returns `true` if semantic context should be used for linting.
+  # TODO: should this be an enum for chosing between top-level and compile?
+  property? semantic = false
+
   # Returns a filename if reading source file from STDIN.
   property stdin_filename : String?
 

src/ameba/environment.cr

@@ -0,0 +1,25 @@
+# Need to point to the stdlib of the current Crystal installation.
+# TODO: verify this works on Windows
+#
+# https://github.com/elbywan/crystalline/blob/bfb0b3b912c64f4f316d56a8e7481261c0edfdca/src/crystalline/main.cr#L25
+module Ameba::EnvironmentConfig
+  # Add the `crystal env` environment variables to the current env.
+  def self.run : Nil
+    initialize_from_crystal_env.each do |k, v|
+      ENV[k] = v
+    end
+  end
+
+  private def self.initialize_from_crystal_env
+    crystal_env
+      .lines
+      .map(&.split('='))
+      .to_h
+  end
+
+  private def self.crystal_env
+    String.build do |io|
+      Process.run("crystal", ["env"], output: io)
+    end
+  end
+end

src/ameba/rule/base.cr

@@ -39,6 +39,13 @@ module Ameba::Rule
       AST::NodeVisitor.new self, source
     end
 
+    # This method can be overridden by rules if they wish to add support for using
+    # semantic information when it is available. By default, the semantic information
+    # is ignored to preserve compatibility with parse-only rules.
+    def test(source : Source, context : SemanticContext?)
+      test(source)
+    end
+
     # NOTE: Can't be abstract
     def test(source : Source, node : Crystal::ASTNode, *opts)
     end
@@ -50,8 +57,8 @@ module Ameba::Rule
     # source = MyRule.new.catch(source)
     # source.valid?
     # ```
-    def catch(source : Source)
-      source.tap { test source }
+    def catch(source : Source, context : SemanticContext? = nil)
+      source.tap { test source, context }
     end
 
     # Returns a name of this rule, which is basically a class name.

src/ameba/rule/lint/no_semantic_information.cr

@@ -0,0 +1,37 @@
+module Ameba::Rule::Lint
+  # A rule that reports when there's no semantic information available for a given type.
+  # This usually happens when there's a file in the codebase not covered by an entrypoint.
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Lint/NoSemanticInformation:
+  #   Enabled: true
+  # ```
+  class NoSemanticInformation < Base
+    properties do
+      since_version "1.7.0"
+      description "Reports types that don't have any semantic information available"
+      severity :warning
+    end
+
+    MSG = "This type doesn't have any semantic information (double check the ameba entrypoints)"
+
+    def test(source, context : SemanticContext?)
+      return if context.nil?
+
+      AST::SemanticVisitor.new self, source, context
+    end
+
+    def test(
+      source,
+      node : Crystal::ClassDef | Crystal::ModuleDef |
+             Crystal::LibDef | Crystal::EnumDef,
+      current_type : Crystal::Type,
+    )
+      return if current_type.lookup_type?(node.name)
+
+      issue_for node.name, MSG
+    end
+  end
+end

src/ameba/rule/lint/semantic.cr

@@ -0,0 +1,34 @@
+module Ameba::Rule::Lint
+  # A rule that reports invalid Crystal semantics.
+  class Semantic < Base
+    properties do
+      since_version "1.7.0"
+      description "Reports invalid Crystal semantics"
+      severity :error
+    end
+
+    def test(source, context : SemanticContext?)
+      return
+    end
+
+    def test(source, sources : Array(Source)) : SemanticContext?
+      SemanticContext.for_entrypoint([source])
+    rescue ex : Crystal::TypeException
+      # TODO: other exception types
+
+      # Attach to the entrypoint if it's not one of our sources
+      source = sources.find(source) { |i| i.path == ex.@filename }
+      filename = ex.@filename || source.path
+
+      location = Crystal::Location.new(
+        filename: filename,
+        line_number: ex.line_number || 0,
+        column_number: ex.column_number
+      )
+
+      issue_for location, location, ex.message.to_s
+
+      nil
+    end
+  end
+end

src/ameba/rule/lint/unknown_type.cr

@@ -0,0 +1,109 @@
+module Ameba::Rule::Lint
+  # A rule that uses semantic information to check parameter and type declaration restrictions
+  # to ensure that those types exist within the current semantic context, even if the
+  # code isn't used during compilation.
+  #
+  # By default, the Crystal compiler does not do any semantic analysis to code that isn't used.
+  # That can lead to issues like this, where a typo can cause unexpected behavior:
+  #
+  # ```
+  # class MessageHandler
+  #   def on_request(msg)
+  #     # generic message handling
+  #   end
+
+  #   def on_request(msg : SpecificMessageType)
+  #     # specific message handling
+  #   end
+  # end
+  # ```
+  #
+  # If `SpecificMessageType` is misspelled, this code will compile and execute normally, but
+  # the method overloading won't happen for the specific message type.
+  #
+  # This is considered invalid:
+  #
+  # ```
+  # count : Int3 = 1
+  #
+  # def hello(name : Str)
+  #   puts "Hello, #{name}!"
+  # end
+  # ```
+  #
+  # And this is considered valid:
+  #
+  # ```
+  # count : Int32 = 1
+  #
+  # def hello(name : String)
+  #   puts "Hello, #{name}!"
+  # end
+  # ```
+  #
+  # YAML configuration example:
+  #
+  # ```
+  # Lint/UnknownType:
+  #   Enabled: true
+  # ```
+  class UnknownType < Base
+    properties do
+      since_version "1.7.0"
+      description "Reports unknown types"
+      severity :error
+    end
+
+    MSG = "Unknown type"
+
+    def test(source, context : SemanticContext?)
+      return if context.nil?
+
+      AST::SemanticVisitor.new self, source, context
+    end
+
+    def test(source, node : Crystal::TypeDeclaration, current_type : Crystal::Type)
+      return if current_type.lookup_type?(node.declared_type)
+
+      validate_type(source, node.declared_type, current_type)
+    end
+
+    def test(source, node : Crystal::Arg, current_type : Crystal::Type)
+      return if (restriction = node.restriction).nil?
+
+      validate_type(source, restriction, current_type)
+    end
+
+    private def validate_type(source, node : Crystal::ASTNode, current_type : Crystal::Type) : Nil
+      case node
+      when Crystal::Path
+        return if current_type.lookup_type?(node)
+
+        issue_for node, MSG
+      when Crystal::Union
+        node.types.each do |type|
+          validate_type(source, type, current_type)
+        end
+      when Crystal::ProcNotation
+        node.inputs.try &.each { |i| validate_type(source, i, current_type) }
+        node.output.try { |i| validate_type(source, i, current_type) }
+      when Crystal::TypeOf
+        node.expressions.each do |type|
+          validate_type(source, type, current_type)
+        end
+      when Crystal::Generic
+        validate_type(source, node.name, current_type)
+
+        node.type_vars.each do |type|
+          validate_type(source, type, current_type)
+        end
+
+        node.named_args.try &.each do |arg|
+          validate_type(source, arg.value, current_type)
+        end
+      when Crystal::Underscore, Crystal::Self
+        # Okay
+      end
+    end
+  end
+end

src/ameba/rule/performance/base.cr

@@ -3,7 +3,7 @@ require "../base"
 module Ameba::Rule::Performance
   # A general base class for performance rules.
   abstract class Base < Ameba::Rule::Base
-    def catch(source : Source)
+    def catch(source : Source, context : SemanticContext? = nil)
       source.spec? ? source : super
     end
   end