diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 86049975..d51482cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,4 +36,4 @@ jobs: run: shards build -Dpreview_mt - name: Run ameba linter - run: bin/ameba --all + run: bin/ameba diff --git a/Makefile b/Makefile index 8b7772a4..f59b75f3 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ build: .PHONY: lint lint: build - ./bin/ameba --all + ./bin/ameba .PHONY: spec spec: diff --git a/README.md b/README.md index 34519df5..d09d58cc 100644 --- a/README.md +++ b/README.md @@ -99,15 +99,6 @@ $ ameba --explain crystal/command/format.cr:26:83 # same thing ### Run in parallel -Starting from 0.31.0 Crystal [supports parallelism](https://crystal-lang.org/2019/09/06/parallelism-in-crystal.html). -It allows to run linting in parallel too. -In order to take advantage of this feature you need to build ameba with preview_mt support: - -```sh -$ crystal build src/cli.cr -Dpreview_mt -o bin/ameba -$ make install -``` - Some quick benchmark results measured while running Ameba on Crystal repo: ```sh diff --git a/spec/ameba/ast/visitors/scope_visitor_spec.cr b/spec/ameba/ast/visitors/scope_visitor_spec.cr index a15402ee..4239e55a 100644 --- a/spec/ameba/ast/visitors/scope_visitor_spec.cr +++ b/spec/ameba/ast/visitors/scope_visitor_spec.cr @@ -2,6 +2,17 @@ require "../../../spec_helper" module Ameba::AST describe ScopeVisitor do + {% for type in %w[class module enum].map(&.id) %} + it "creates a scope for the {{ type }} def" do + rule = ScopeRule.new + ScopeVisitor.new rule, Source.new <<-CRYSTAL + {{ type }} Foo + end + CRYSTAL + rule.scopes.size.should eq 1 + end + {% end %} + it "creates a scope for the def" do rule = ScopeRule.new ScopeVisitor.new rule, Source.new <<-CRYSTAL @@ -54,5 +65,33 @@ module Ameba::AST outer_block.outer_scope.should be_nil end end + + context "#visibility" do + it "is being properly set" do + rule = ScopeRule.new + ScopeVisitor.new rule, Source.new <<-CRYSTAL + private class Foo + end + CRYSTAL + rule.scopes.size.should eq 1 + rule.scopes.first.visibility.should eq Crystal::Visibility::Private + end + + it "is being inherited from the outer scope(s)" do + rule = ScopeRule.new + ScopeVisitor.new rule, Source.new <<-CRYSTAL + private class Foo + class Bar + def baz + end + end + end + CRYSTAL + rule.scopes.size.should eq 3 + rule.scopes.each &.visibility.should eq Crystal::Visibility::Private + rule.scopes.last.node.visibility.should eq Crystal::Visibility::Private + rule.scopes[0...-1].each &.node.visibility.should eq Crystal::Visibility::Public + end + end end end diff --git a/spec/ameba/rule/lint/documentation_spec.cr b/spec/ameba/rule/lint/documentation_spec.cr new file mode 100644 index 00000000..442b1444 --- /dev/null +++ b/spec/ameba/rule/lint/documentation_spec.cr @@ -0,0 +1,151 @@ +require "../../../spec_helper" + +module Ameba::Rule::Lint + subject = Documentation.new + .tap(&.ignore_classes = false) + .tap(&.ignore_modules = false) + .tap(&.ignore_enums = false) + .tap(&.ignore_defs = false) + .tap(&.ignore_macros = false) + + describe Documentation do + it "passes for undocumented private types" do + expect_no_issues subject, <<-CRYSTAL + private class Foo + def foo + end + end + + private module Bar + def bar + end + end + + private enum Baz + end + + private def bat + end + + private macro bag + end + CRYSTAL + end + + it "passes for documented public types" do + expect_no_issues subject, <<-CRYSTAL + # Foo + class Foo + # foo + def foo + end + end + + # Bar + module Bar + # bar + def bar + end + end + + # Baz + enum Baz + end + + # bat + def bat + end + + # bag + macro bag + end + CRYSTAL + end + + it "fails if there is an undocumented public type" do + expect_issue subject, <<-CRYSTAL + class Foo + # ^^^^^^^^^ error: Missing documentation + end + + module Bar + # ^^^^^^^^^^ error: Missing documentation + end + + enum Baz + # ^^^^^^^^ error: Missing documentation + end + + def bat + # ^^^^^^^ error: Missing documentation + end + + macro bag + # ^^^^^^^^^ error: Missing documentation + end + CRYSTAL + end + + context "properties" do + describe "#ignore_classes" do + it "lets the rule to ignore method definitions if true" do + rule = Documentation.new + rule.ignore_classes = true + + expect_no_issues rule, <<-CRYSTAL + class Foo + end + CRYSTAL + end + end + + describe "#ignore_modules" do + it "lets the rule to ignore method definitions if true" do + rule = Documentation.new + rule.ignore_modules = true + + expect_no_issues rule, <<-CRYSTAL + module Bar + end + CRYSTAL + end + end + + describe "#ignore_enums" do + it "lets the rule to ignore method definitions if true" do + rule = Documentation.new + rule.ignore_enums = true + + expect_no_issues rule, <<-CRYSTAL + enum Baz + end + CRYSTAL + end + end + + describe "#ignore_defs" do + it "lets the rule to ignore method definitions if true" do + rule = Documentation.new + rule.ignore_defs = true + + expect_no_issues rule, <<-CRYSTAL + def bat + end + CRYSTAL + end + end + + describe "#ignore_macros" do + it "lets the rule to ignore macros if true" do + rule = Documentation.new + rule.ignore_macros = true + + expect_no_issues rule, <<-CRYSTAL + macro bag + end + CRYSTAL + end + end + end + end +end diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr index 4a18746b..365febd1 100644 --- a/spec/spec_helper.cr +++ b/spec/spec_helper.cr @@ -43,6 +43,9 @@ module Ameba description "Internal rule to test scopes" end + def test(source, node : Crystal::VisibilityModifier, scope : AST::Scope) + end + def test(source, node : Crystal::ASTNode, scope : AST::Scope) @scopes << scope end diff --git a/src/ameba/ast/scope.cr b/src/ameba/ast/scope.cr index b74f8b62..9d727409 100644 --- a/src/ameba/ast/scope.cr +++ b/src/ameba/ast/scope.cr @@ -7,6 +7,9 @@ module Ameba::AST # Whether the scope yields. setter yields = false + # Scope visibility level + setter visibility : Crystal::Visibility? + # Link to local variables getter variables = [] of Variable @@ -121,10 +124,7 @@ module Ameba::AST # end # ``` def spawn_block? - return false unless node.is_a?(Crystal::Block) - - call = node.as(Crystal::Block).call - call.try(&.name) == "spawn" + node.as?(Crystal::Block).try(&.call).try(&.name) == "spawn" end # Returns `true` if current scope sits inside a macro. @@ -167,9 +167,12 @@ module Ameba::AST # Returns `true` if current scope (or any of inner scopes) yields, # `false` otherwise. def yields?(check_inner_scopes = true) - return true if @yields - return inner_scopes.any?(&.yields?) if check_inner_scopes - false + @yields || (check_inner_scopes && inner_scopes.any?(&.yields?)) + end + + # Returns visibility of the current scope (could be inherited from the outer scope). + def visibility + @visibility || outer_scope.try(&.visibility) end # Returns `true` if current scope is a def, `false` otherwise. diff --git a/src/ameba/ast/visitors/node_visitor.cr b/src/ameba/ast/visitors/node_visitor.cr index 4e6e3618..315d1b7b 100644 --- a/src/ameba/ast/visitors/node_visitor.cr +++ b/src/ameba/ast/visitors/node_visitor.cr @@ -1,34 +1,6 @@ require "./base_visitor" module Ameba::AST - # List of nodes to be visited by Ameba's rules. - NODES = { - Alias, - IsA, - Assign, - Call, - Block, - Case, - ClassDef, - ClassVar, - Def, - EnumDef, - ExceptionHandler, - Expressions, - HashLiteral, - If, - InstanceVar, - LibDef, - ModuleDef, - NilLiteral, - StringInterpolation, - Unless, - Var, - When, - While, - Until, - } - # An AST Visitor that traverses the source and allows all nodes # to be inspected by rules. # @@ -36,6 +8,34 @@ module Ameba::AST # visitor = Ameba::AST::NodeVisitor.new(rule, source) # ``` class NodeVisitor < BaseVisitor + # List of nodes to be visited by Ameba's rules. + NODES = { + Alias, + IsA, + Assign, + Call, + Block, + Case, + ClassDef, + ClassVar, + Def, + EnumDef, + ExceptionHandler, + Expressions, + HashLiteral, + If, + InstanceVar, + LibDef, + ModuleDef, + NilLiteral, + StringInterpolation, + Unless, + Var, + When, + While, + Until, + } + @skip : Array(Crystal::ASTNode.class)? def initialize(@rule, @source, skip = nil) @@ -43,6 +43,11 @@ module Ameba::AST super @rule, @source end + def visit(node : Crystal::VisibilityModifier) + node.exp.visibility = node.modifier + true + end + {% for name in NODES %} # A visit callback for `Crystal::{{ name }}` node. # diff --git a/src/ameba/ast/visitors/scope_visitor.cr b/src/ameba/ast/visitors/scope_visitor.cr index a5c578ae..28a563ba 100644 --- a/src/ameba/ast/visitors/scope_visitor.cr +++ b/src/ameba/ast/visitors/scope_visitor.cr @@ -25,6 +25,7 @@ module Ameba::AST @scope_queue = [] of Scope @current_scope : Scope @current_assign : Crystal::ASTNode? + @visibility_modifier : Crystal::Visibility? @skip : Array(Crystal::ASTNode.class)? def initialize(@rule, @source, skip = nil) @@ -36,12 +37,18 @@ module Ameba::AST private def on_scope_enter(node) return if skip?(node) - @current_scope = Scope.new(node, @current_scope) + + scope = Scope.new(node, @current_scope) + scope.visibility = @visibility_modifier + + @current_scope = scope end private def on_scope_end(node) @scope_queue << @current_scope + @visibility_modifier = nil + # go up if this is not a top level scope return unless outer_scope = @current_scope.outer_scope @current_scope = outer_scope @@ -64,6 +71,12 @@ module Ameba::AST end {% end %} + # :nodoc: + def visit(node : Crystal::VisibilityModifier) + @visibility_modifier = node.exp.visibility = node.modifier + true + end + # :nodoc: def visit(node : Crystal::Yield) @current_scope.yields = true @@ -109,6 +122,7 @@ module Ameba::AST @current_assign = node.value unless node.value.nil? end + # :nodoc: def end_visit(node : Crystal::TypeDeclaration) return unless (var = node.var).is_a?(Crystal::Var) diff --git a/src/ameba/config.cr b/src/ameba/config.cr index ff039f28..5b426195 100644 --- a/src/ameba/config.cr +++ b/src/ameba/config.cr @@ -240,6 +240,7 @@ class Ameba::Config # :nodoc: module RuleConfig + # Define rule properties macro properties(&block) {% definitions = [] of NamedTuple %} {% if block.body.is_a? Assign %} diff --git a/src/ameba/rule/base.cr b/src/ameba/rule/base.cr index 675ae99b..7d656d8e 100644 --- a/src/ameba/rule/base.cr +++ b/src/ameba/rule/base.cr @@ -112,6 +112,7 @@ module Ameba::Rule name.hash end + # Adds an issue to the *source* macro issue_for(*args, **kwargs, &block) source.add_issue(self, {{ *args }}, {{ **kwargs }}) {{ block }} end diff --git a/src/ameba/rule/lint/documentation.cr b/src/ameba/rule/lint/documentation.cr new file mode 100644 index 00000000..5e6d134b --- /dev/null +++ b/src/ameba/rule/lint/documentation.cr @@ -0,0 +1,66 @@ +module Ameba::Rule::Lint + # A rule that enforces documentation for public types: + # modules, classes, enums, methods and macros. + # + # YAML configuration example: + # + # ``` + # Lint/Documentation: + # Enabled: true + # ``` + class Documentation < Base + properties do + enabled false + description "Enforces public types to be documented" + + ignore_classes false + ignore_modules true + ignore_enums false + ignore_defs true + ignore_macros false + end + + MSG = "Missing documentation" + + MACRO_HOOK_NAMES = %w[ + inherited + included extended + method_missing method_added + finished + ] + + def test(source) + AST::ScopeVisitor.new self, source + end + + def test(source, node : Crystal::ClassDef, scope : AST::Scope) + ignore_classes? || check_missing_doc(source, node, scope) + end + + def test(source, node : Crystal::ModuleDef, scope : AST::Scope) + ignore_modules? || check_missing_doc(source, node, scope) + end + + def test(source, node : Crystal::EnumDef, scope : AST::Scope) + ignore_enums? || check_missing_doc(source, node, scope) + end + + def test(source, node : Crystal::Def, scope : AST::Scope) + ignore_defs? || check_missing_doc(source, node, scope) + end + + def test(source, node : Crystal::Macro, scope : AST::Scope) + node.name.in?(MACRO_HOOK_NAMES) || + ignore_macros? || check_missing_doc(source, node, scope) + end + + private def check_missing_doc(source, node, scope) + visibility = scope.visibility + + return if visibility && !visibility.public? + return if node.doc.presence + + issue_for(node, MSG) + end + end +end