mirror of
				https://gitea.invidious.io/iv-org/shard-ameba.git
				synced 2024-08-15 00:53:29 +00:00 
			
		
		
		
	Merge pull request #207 from crystal-ameba/release/0.14.0
Release 0.14.0
This commit is contained in:
		
						commit
						d8c32f0045
					
				
					 106 changed files with 2045 additions and 448 deletions
				
			
		
							
								
								
									
										10
									
								
								README.md
									
										
									
									
									
								
							
							
						
						
									
										10
									
								
								README.md
									
										
									
									
									
								
							|  | @ -4,7 +4,7 @@ | |||
|   <p align="center">Code style linter for Crystal<p> | ||||
|   <p align="center"> | ||||
|     <sup> | ||||
|       <i> (a single-celled animal that catches food and moves about by extending fingerlike projections of protoplasm) </i> | ||||
|       <i>(a single-celled animal that catches food and moves about by extending fingerlike projections of protoplasm)</i> | ||||
|     </sup> | ||||
|   </p> | ||||
|   <p align="center"> | ||||
|  | @ -35,7 +35,7 @@ | |||
| ## About | ||||
| 
 | ||||
| Ameba is a static code analysis tool for the Crystal language. | ||||
| It enforces a consistent [Crystal code style](https://crystal-lang.org/docs/conventions/coding_style.html), | ||||
| It enforces a consistent [Crystal code style](https://crystal-lang.org/reference/conventions/coding_style.html), | ||||
| also catches code smells and wrong code constructions. | ||||
| 
 | ||||
| See also [Roadmap](https://github.com/crystal-ameba/ameba/wiki). | ||||
|  | @ -46,7 +46,7 @@ Run `ameba` binary within your project directory to catch code issues: | |||
| 
 | ||||
| ```sh | ||||
| $ ameba | ||||
| Inspecting 107 files. | ||||
| Inspecting 107 files | ||||
| 
 | ||||
| ...............F.....................F.................................................................... | ||||
| 
 | ||||
|  | @ -61,9 +61,7 @@ src/ameba/formatter/base_formatter.cr:12:7 | |||
|          ^ | ||||
| 
 | ||||
| Finished in 542.64 milliseconds | ||||
| 
 | ||||
| 129 inspected, 2 failures. | ||||
| 
 | ||||
| 129 inspected, 2 failures | ||||
| ``` | ||||
| 
 | ||||
| ### Run in parallel | ||||
|  |  | |||
|  | @ -1,5 +1,5 @@ | |||
| name: ameba | ||||
| version: 0.13.4 | ||||
| version: 0.14.0 | ||||
| 
 | ||||
| authors: | ||||
|   - Vitalii Elenhaupt <velenhaupt@gmail.com> | ||||
|  |  | |||
|  | @ -113,7 +113,7 @@ module Ameba::AST | |||
|     it "adds a new variable to the scope" do | ||||
|       scope = Scope.new as_node("") | ||||
|       scope.add_variable(Crystal::Var.new "foo") | ||||
|       scope.variables.any?.should be_true | ||||
|       scope.variables.empty?.should be_false | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|  |  | |||
|  | @ -48,7 +48,7 @@ module Ameba::AST | |||
|       it "assigns the variable (creates a new assignment)" do | ||||
|         variable = Variable.new(var_node, scope) | ||||
|         variable.assign(assign_node, scope) | ||||
|         variable.assignments.any?.should be_true | ||||
|         variable.assignments.empty?.should be_false | ||||
|       end | ||||
| 
 | ||||
|       it "can create multiple assignments" do | ||||
|  | @ -64,7 +64,7 @@ module Ameba::AST | |||
|         variable = Variable.new(var_node, scope) | ||||
|         variable.assign(as_node("foo=1"), scope) | ||||
|         variable.reference(var_node, scope) | ||||
|         variable.references.any?.should be_true | ||||
|         variable.references.empty?.should be_false | ||||
|       end | ||||
| 
 | ||||
|       it "adds a reference to the scope" do | ||||
|  |  | |||
							
								
								
									
										17
									
								
								spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										17
									
								
								spec/ameba/ast/visitors/top_level_nodes_visitor_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,17 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::AST | ||||
|   describe TopLevelNodesVisitor do | ||||
|     describe "#require_nodes" do | ||||
|       it "returns require node" do | ||||
|         source = Source.new %( | ||||
|           require "foo" | ||||
|           def bar; end | ||||
|         ) | ||||
|         visitor = TopLevelNodesVisitor.new(source.ast) | ||||
|         visitor.require_nodes.size.should eq 1 | ||||
|         visitor.require_nodes.first.to_s.should eq %q(require "foo") | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -42,6 +42,16 @@ module Ameba::Cli | |||
|         c.except.should eq %w(RULE1 RULE2) | ||||
|       end | ||||
| 
 | ||||
|       it "defaults rules? flag to false" do | ||||
|         c = Cli.parse_args %w(file.cr) | ||||
|         c.rules?.should eq false | ||||
|       end | ||||
| 
 | ||||
|       it "accepts --rules flag" do | ||||
|         c = Cli.parse_args %w(--rules) | ||||
|         c.rules?.should eq true | ||||
|       end | ||||
| 
 | ||||
|       it "defaults all? flag to false" do | ||||
|         c = Cli.parse_args %w(file.cr) | ||||
|         c.all?.should eq false | ||||
|  |  | |||
|  | @ -120,7 +120,7 @@ module Ameba | |||
|       it "returns list of sources" do | ||||
|         config.sources.size.should be > 0 | ||||
|         config.sources.first.should be_a Source | ||||
|         config.sources.any? { |s| s.fullpath == __FILE__ }.should be_true | ||||
|         config.sources.any?(&.fullpath.==(__FILE__)).should be_true | ||||
|       end | ||||
| 
 | ||||
|       it "returns a list of sources mathing globs" do | ||||
|  | @ -130,7 +130,7 @@ module Ameba | |||
| 
 | ||||
|       it "returns a lisf of sources excluding 'Excluded'" do | ||||
|         config.excluded = %w(**/config_spec.cr) | ||||
|         config.sources.any? { |s| s.fullpath == __FILE__ }.should be_false | ||||
|         config.sources.any?(&.fullpath.==(__FILE__)).should be_false | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  |  | |||
|  | @ -8,7 +8,7 @@ module Ameba::Formatter | |||
|     describe "#started" do | ||||
|       it "writes started message" do | ||||
|         subject.started [Source.new ""] | ||||
|         output.to_s.should eq "Inspecting 1 file.\n\n" | ||||
|         output.to_s.should eq "Inspecting 1 file\n\n" | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  | @ -29,7 +29,7 @@ module Ameba::Formatter | |||
|     describe "#finished" do | ||||
|       it "writes a final message" do | ||||
|         subject.finished [Source.new ""] | ||||
|         output.to_s.should contain "1 inspected, 0 failures." | ||||
|         output.to_s.should contain "1 inspected, 0 failures" | ||||
|       end | ||||
| 
 | ||||
|       it "writes the elapsed time" do | ||||
|  | @ -45,7 +45,7 @@ module Ameba::Formatter | |||
|           end | ||||
|           subject.finished [s] | ||||
|           log = output.to_s | ||||
|           log.should contain "1 inspected, 2 failures." | ||||
|           log.should contain "1 inspected, 2 failures" | ||||
|           log.should contain "DummyRuleError" | ||||
|           log.should contain "NamedRuleError" | ||||
|         end | ||||
|  | @ -60,7 +60,7 @@ module Ameba::Formatter | |||
|           end | ||||
|           subject.finished [s] | ||||
|           log = output.to_s | ||||
|           log.should contain "> a = 22" | ||||
|           log.should contain "> \e[97ma = 22" | ||||
|           log.should contain "      \e[33m^\e[0m" | ||||
|         end | ||||
| 
 | ||||
|  | @ -99,7 +99,7 @@ module Ameba::Formatter | |||
|           s.add_issue(DummyRule.new, location: {1, 1}, | ||||
|             message: "DummyRuleError", status: :disabled) | ||||
|           subject.finished [s] | ||||
|           output.to_s.should contain "1 inspected, 0 failures." | ||||
|           output.to_s.should contain "1 inspected, 0 failures" | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  |  | |||
|  | @ -8,6 +8,65 @@ module Ameba::Formatter | |||
|   subject = Subject.new | ||||
| 
 | ||||
|   describe Util do | ||||
|     describe "#deansify" do | ||||
|       it "returns given string without ANSI codes" do | ||||
|         str = String.build do |io| | ||||
|           io << "foo".colorize.green.underline | ||||
|           io << '-' | ||||
|           io << "bar".colorize.red.underline | ||||
|         end | ||||
|         subject.deansify("foo-bar").should eq "foo-bar" | ||||
|         subject.deansify(str).should eq "foo-bar" | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     describe "#trim" do | ||||
|       it "trims string longer than :max_length" do | ||||
|         subject.trim(("+" * 300), 1).should eq "+" | ||||
|         subject.trim(("+" * 300), 3).should eq "+++" | ||||
|         subject.trim(("+" * 300), 5).should eq "+ ..." | ||||
|         subject.trim(("+" * 300), 7).should eq "+++ ..." | ||||
|       end | ||||
| 
 | ||||
|       it "leaves intact string shorter than :max_length" do | ||||
|         subject.trim(("+" * 3), 100).should eq "+++" | ||||
|       end | ||||
| 
 | ||||
|       it "allows to use custom ellipsis" do | ||||
|         subject.trim(("+" * 300), 3, "…").should eq "++…" | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     describe "#context" do | ||||
|       it "returns correct pre/post context lines" do | ||||
|         source = Source.new <<-EOF | ||||
|           # pre:1 | ||||
|             # pre:2 | ||||
|               # pre:3 | ||||
|                 # pre:4 | ||||
|                   # pre:5 | ||||
|           a = 1 | ||||
|                   # post:1 | ||||
|                 # post:2 | ||||
|               # post:3 | ||||
|             # post:4 | ||||
|           # post:5 | ||||
|           EOF | ||||
| 
 | ||||
|         subject.context(source.lines, lineno: 6, context_lines: 3) | ||||
|           .should eq({<<-PRE.lines, <<-POST.lines | ||||
|                 # pre:3 | ||||
|                   # pre:4 | ||||
|                     # pre:5 | ||||
|             PRE | ||||
|                     # post:1 | ||||
|                   # post:2 | ||||
|                 # post:3 | ||||
|             POST | ||||
|           }) | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     describe "#affected_code" do | ||||
|       it "returns nil if there is no such a line number" do | ||||
|         source = Source.new %( | ||||
|  | @ -22,7 +81,38 @@ module Ameba::Formatter | |||
|           a = 1 | ||||
|         ) | ||||
|         location = Crystal::Location.new("filename", 1, 1) | ||||
|         subject.affected_code(source, location).should eq "> a = 1\n  \e[33m^\e[0m" | ||||
|         subject.deansify(subject.affected_code(source, location)) | ||||
|           .should eq "> a = 1\n  ^\n" | ||||
|       end | ||||
| 
 | ||||
|       it "returns correct line if it is found" do | ||||
|         source = Source.new <<-EOF | ||||
|           # pre:1 | ||||
|             # pre:2 | ||||
|               # pre:3 | ||||
|                 # pre:4 | ||||
|                   # pre:5 | ||||
|           a = 1 | ||||
|                   # post:1 | ||||
|                 # post:2 | ||||
|               # post:3 | ||||
|             # post:4 | ||||
|           # post:5 | ||||
|           EOF | ||||
| 
 | ||||
|         location = Crystal::Location.new("filename", 6, 1) | ||||
|         subject.deansify(subject.affected_code(source, location, context_lines: 3)) | ||||
|           .should eq <<-STR | ||||
|             >     # pre:3 | ||||
|             >       # pre:4 | ||||
|             >         # pre:5 | ||||
|             > a = 1 | ||||
|               ^ | ||||
|             >         # post:1 | ||||
|             >       # post:2 | ||||
|             >     # post:3 | ||||
| 
 | ||||
|             STR | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -42,9 +42,22 @@ module Ameba | |||
|         location: nil, | ||||
|         end_location: nil, | ||||
|         message: "", | ||||
|         status: :enabled | ||||
|         status: :disabled | ||||
| 
 | ||||
|       issue.status.should eq :enabled | ||||
|       issue.status.should eq Issue::Status::Disabled | ||||
|       issue.disabled?.should be_true | ||||
|       issue.enabled?.should be_false | ||||
|     end | ||||
| 
 | ||||
|     it "sets status to :enabled by default" do | ||||
|       issue = Issue.new rule: DummyRule.new, | ||||
|         location: nil, | ||||
|         end_location: nil, | ||||
|         message: "" | ||||
| 
 | ||||
|       issue.status.should eq Issue::Status::Enabled | ||||
|       issue.enabled?.should be_true | ||||
|       issue.disabled?.should be_false | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
							
								
								
									
										48
									
								
								spec/ameba/rule/lint/duplicated_require_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										48
									
								
								spec/ameba/rule/lint/duplicated_require_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,48 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Lint | ||||
|   subject = DuplicatedRequire.new | ||||
| 
 | ||||
|   describe DuplicatedRequire do | ||||
|     it "passes if there are no duplicated requires" do | ||||
|       source = Source.new %( | ||||
|         require "math" | ||||
|         require "big" | ||||
|         require "big/big_decimal" | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there are a duplicated requires" do | ||||
|       source = Source.new %( | ||||
|         require "big" | ||||
|         require "math" | ||||
|         require "big" | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       source = Source.new %( | ||||
|         require "./thing" | ||||
|         require "./thing" | ||||
|         require "./another_thing" | ||||
|         require "./another_thing" | ||||
|       ), "source.cr" | ||||
| 
 | ||||
|       subject.catch(source).should_not be_valid | ||||
| 
 | ||||
|       issue = source.issues.first | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:2:1" | ||||
|       issue.end_location.to_s.should eq "" | ||||
|       issue.message.should eq "Duplicated require of `./thing`" | ||||
| 
 | ||||
|       issue = source.issues.last | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:4:1" | ||||
|       issue.end_location.to_s.should eq "" | ||||
|       issue.message.should eq "Duplicated require of `./another_thing`" | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -77,7 +77,7 @@ module Ameba::Rule::Lint | |||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:2:14" | ||||
|       issue.end_location.to_s.should eq "source.cr:2:30" | ||||
|       issue.message.should eq "Use each instead of each_with_object" | ||||
|       issue.message.should eq "Use `each` instead of `each_with_object`" | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
							
								
								
									
										127
									
								
								spec/ameba/rule/lint/spec_focus_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										127
									
								
								spec/ameba/rule/lint/spec_focus_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,127 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Lint | ||||
|   describe SpecFocus do | ||||
|     subject = SpecFocus.new | ||||
| 
 | ||||
|     it "does not report if spec is not focused" do | ||||
|       s = Source.new %( | ||||
|         context "context" {} | ||||
|         describe "describe" {} | ||||
|         it "it" {} | ||||
|         pending "pending" {} | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is a focused context" do | ||||
|       s = Source.new %( | ||||
|         context "context", focus: true do | ||||
|         end | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is a focused describe block" do | ||||
|       s = Source.new %( | ||||
|         describe "describe", focus: true do | ||||
|         end | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is a focused it block" do | ||||
|       s = Source.new %( | ||||
|         it "it", focus: true do | ||||
|         end | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is a focused pending block" do | ||||
|       s = Source.new %( | ||||
|         pending "pending", focus: true do | ||||
|         end | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is a spec item with `focus: false`" do | ||||
|       s = Source.new %( | ||||
|         it "it", focus: false do | ||||
|         end | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "does not report if there is non spec block with :focus" do | ||||
|       s = Source.new %( | ||||
|         some_method "foo", focus: true do | ||||
|         end | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "does not report if there is a tagged item with :focus" do | ||||
|       s = Source.new %( | ||||
|         it "foo", tags: "focus" do | ||||
|         end | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "does not report if there are focused spec items without blocks" do | ||||
|       s = Source.new %( | ||||
|         describe "foo", focus: true | ||||
|         context "foo", focus: true | ||||
|         it "foo", focus: true | ||||
|         pending "foo", focus: true | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "does not report if there are focused items out of spec file" do | ||||
|       s = Source.new %( | ||||
|         describe "foo", focus: true {} | ||||
|         context "foo", focus: true {} | ||||
|         it "foo", focus: true {} | ||||
|         pending "foo", focus: true {} | ||||
|       ) | ||||
| 
 | ||||
|       subject.catch(s).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       s = Source.new %( | ||||
|         it "foo", focus: true do | ||||
|           it "bar", focus: true {} | ||||
|         end | ||||
|       ), path: "source_spec.cr" | ||||
| 
 | ||||
|       subject.catch(s).should_not be_valid | ||||
| 
 | ||||
|       s.issues.size.should eq(2) | ||||
| 
 | ||||
|       first, second = s.issues | ||||
| 
 | ||||
|       first.rule.should_not be_nil | ||||
|       first.location.to_s.should eq "source_spec.cr:1:11" | ||||
|       first.end_location.to_s.should eq "" | ||||
|       first.message.should eq "Focused spec item detected" | ||||
| 
 | ||||
|       second.rule.should_not be_nil | ||||
|       second.location.to_s.should eq "source_spec.cr:2:13" | ||||
|       second.end_location.to_s.should eq "" | ||||
|       second.message.should eq "Focused spec item detected" | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										46
									
								
								spec/ameba/rule/performance/any_instead_of_empty_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										46
									
								
								spec/ameba/rule/performance/any_instead_of_empty_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,46 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Performance | ||||
|   subject = AnyInsteadOfEmpty.new | ||||
| 
 | ||||
|   describe AnyInsteadOfEmpty do | ||||
|     it "passes if there is no potential performance improvements" do | ||||
|       source = Source.new %( | ||||
|         [1, 2, 3].any?(&.zero?) | ||||
|         [1, 2, 3].any?(String) | ||||
|         [1, 2, 3].any?(1..3) | ||||
|         [1, 2, 3].any? { |e| e > 1 } | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is any? call without a block nor argument" do | ||||
|       source = Source.new %( | ||||
|         [1, 2, 3].any? | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     context "macro" do | ||||
|       it "reports in macro scope" do | ||||
|         source = Source.new %( | ||||
|           {{ [1, 2, 3].any? }} | ||||
|         ) | ||||
|         subject.catch(source).should_not be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       source = Source.new path: "source.cr", code: %( | ||||
|         [1, 2, 3].any? | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|       issue = source.issues.first | ||||
| 
 | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:1:11" | ||||
|       issue.end_location.to_s.should eq "source.cr:1:15" | ||||
|       issue.message.should eq "Use `!{...}.empty?` instead of `{...}.any?`" | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -0,0 +1,69 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Performance | ||||
|   subject = ChainedCallWithNoBang.new | ||||
| 
 | ||||
|   describe ChainedCallWithNoBang do | ||||
|     it "passes if there is no potential performance improvements" do | ||||
|       source = Source.new %( | ||||
|         (1..3).select { |e| e > 1 }.sort! | ||||
|         (1..3).select { |e| e > 1 }.sort_by!(&.itself) | ||||
|         (1..3).select { |e| e > 1 }.uniq! | ||||
|         (1..3).select { |e| e > 1 }.shuffle! | ||||
|         (1..3).select { |e| e > 1 }.reverse! | ||||
|         (1..3).select { |e| e > 1 }.rotate! | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is select followed by reverse" do | ||||
|       source = Source.new %( | ||||
|         [1, 2, 3].select { |e| e > 1 }.reverse | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is select followed by reverse followed by other call" do | ||||
|       source = Source.new %( | ||||
|         [1, 2, 3].select { |e| e > 2 }.reverse.size | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     context "properties" do | ||||
|       it "allows to configure `call_names`" do | ||||
|         source = Source.new %( | ||||
|           [1, 2, 3].select { |e| e > 2 }.reverse | ||||
|         ) | ||||
|         rule = ChainedCallWithNoBang.new | ||||
|         rule.call_names = %w(uniq) | ||||
|         rule.catch(source).should be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       source = Source.new path: "source.cr", code: <<-CODE | ||||
|         [1, 2, 3].select { |e| e > 1 }.reverse | ||||
|         CODE | ||||
| 
 | ||||
|       subject.catch(source).should_not be_valid | ||||
|       source.issues.size.should eq 1 | ||||
| 
 | ||||
|       issue = source.issues.first | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:1:32" | ||||
|       issue.end_location.to_s.should eq "source.cr:1:39" | ||||
| 
 | ||||
|       issue.message.should eq "Use bang method variant `reverse!` after chained `select` call" | ||||
|     end | ||||
| 
 | ||||
|     context "macro" do | ||||
|       it "doesn't report in macro scope" do | ||||
|         source = Source.new %( | ||||
|           {{ [1, 2, 3].select { |e| e > 2  }.reverse }} | ||||
|         ) | ||||
|         subject.catch(source).should be_valid | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										50
									
								
								spec/ameba/rule/performance/compact_after_map_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										50
									
								
								spec/ameba/rule/performance/compact_after_map_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,50 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Performance | ||||
|   subject = CompactAfterMap.new | ||||
| 
 | ||||
|   describe CompactAfterMap do | ||||
|     it "passes if there is no potential performance improvements" do | ||||
|       source = Source.new %( | ||||
|         (1..3).compact_map(&.itself) | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "passes if there is map followed by a bang call" do | ||||
|       source = Source.new %( | ||||
|         (1..3).map(&.itself).compact! | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is map followed by compact call" do | ||||
|       source = Source.new %( | ||||
|         (1..3).map(&.itself).compact | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     context "macro" do | ||||
|       it "doesn't report in macro scope" do | ||||
|         source = Source.new %( | ||||
|           {{ [1, 2, 3].map(&.to_s).compact }} | ||||
|         ) | ||||
|         subject.catch(source).should be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       s = Source.new %( | ||||
|         (1..3).map(&.itself).compact | ||||
|       ), "source.cr" | ||||
|       subject.catch(s).should_not be_valid | ||||
|       issue = s.issues.first | ||||
| 
 | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:1:8" | ||||
|       issue.end_location.to_s.should eq "source.cr:1:29" | ||||
|       issue.message.should eq "Use `compact_map {...}` instead of `map {...}.compact`" | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										43
									
								
								spec/ameba/rule/performance/flatten_after_map_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										43
									
								
								spec/ameba/rule/performance/flatten_after_map_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,43 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Performance | ||||
|   subject = FlattenAfterMap.new | ||||
| 
 | ||||
|   describe FlattenAfterMap do | ||||
|     it "passes if there is no potential performance improvements" do | ||||
|       source = Source.new %( | ||||
|         %w[Alice Bob].flat_map(&.chars) | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is map followed by flatten call" do | ||||
|       source = Source.new %( | ||||
|         %w[Alice Bob].map(&.chars).flatten | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     context "macro" do | ||||
|       it "doesn't report in macro scope" do | ||||
|         source = Source.new %( | ||||
|           {{ %w[Alice Bob].map(&.chars).flatten }} | ||||
|         ) | ||||
|         subject.catch(source).should be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       s = Source.new %( | ||||
|         %w[Alice Bob].map(&.chars).flatten | ||||
|       ), "source.cr" | ||||
|       subject.catch(s).should_not be_valid | ||||
|       issue = s.issues.first | ||||
| 
 | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:1:15" | ||||
|       issue.end_location.to_s.should eq "source.cr:1:35" | ||||
|       issue.message.should eq "Use `flat_map {...}` instead of `map {...}.flatten`" | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										59
									
								
								spec/ameba/rule/performance/map_instead_of_block_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										59
									
								
								spec/ameba/rule/performance/map_instead_of_block_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,59 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Performance | ||||
|   subject = MapInsteadOfBlock.new | ||||
| 
 | ||||
|   describe MapInsteadOfBlock do | ||||
|     it "passes if there is no potential performance improvements" do | ||||
|       source = Source.new %( | ||||
|         (1..3).join(&.to_s) | ||||
|         (1..3).sum(&.*(2)) | ||||
|         (1..3).product(&.*(2)) | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is map followed by join without a block" do | ||||
|       source = Source.new %( | ||||
|         (1..3).map(&.to_s).join | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is map followed by join without a block (with argument)" do | ||||
|       source = Source.new %( | ||||
|         (1..3).map(&.to_s).join('.') | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is map followed by join with a block" do | ||||
|       source = Source.new %( | ||||
|         (1..3).map(&.to_s).join(&.itself) | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     context "macro" do | ||||
|       it "doesn't report in macro scope" do | ||||
|         source = Source.new %( | ||||
|           {{ [1, 2, 3].map(&.to_s).join }} | ||||
|         ) | ||||
|         subject.catch(source).should be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       s = Source.new %( | ||||
|         (1..3).map(&.to_s).join | ||||
|       ), "source.cr" | ||||
|       subject.catch(s).should_not be_valid | ||||
|       issue = s.issues.first | ||||
| 
 | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:1:8" | ||||
|       issue.end_location.to_s.should eq "source.cr:1:24" | ||||
|       issue.message.should eq "Use `join {...}` instead of `map {...}.join`" | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										64
									
								
								spec/ameba/rule/style/is_a_filter_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										64
									
								
								spec/ameba/rule/style/is_a_filter_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,64 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Style | ||||
|   subject = IsAFilter.new | ||||
| 
 | ||||
|   describe IsAFilter do | ||||
|     it "passes if there is no potential performance improvements" do | ||||
|       source = Source.new %( | ||||
|         [1, 2, nil].select(Int32) | ||||
|         [1, 2, nil].reject(Nil) | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is .is_a? call within select" do | ||||
|       source = Source.new %( | ||||
|         [1, 2, nil].select(&.is_a?(Int32)) | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is .nil? call within reject" do | ||||
|       source = Source.new %( | ||||
|         [1, 2, nil].reject(&.nil?) | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     context "properties" do | ||||
|       it "allows to configure filter_names" do | ||||
|         source = Source.new %( | ||||
|           [1, 2, nil].reject(&.nil?) | ||||
|         ) | ||||
|         rule = IsAFilter.new | ||||
|         rule.filter_names = %w(select) | ||||
|         rule.catch(source).should be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context "macro" do | ||||
|       it "reports in macro scope" do | ||||
|         source = Source.new %( | ||||
|           {{ [1, 2, nil].reject(&.nil?) }} | ||||
|         ) | ||||
|         subject.catch(source).should_not be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       source = Source.new path: "source.cr", code: %( | ||||
|         [1, 2, nil].reject(&.nil?) | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|       source.issues.size.should eq 1 | ||||
| 
 | ||||
|       issue = source.issues.first | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:1:13" | ||||
|       issue.end_location.to_s.should eq "source.cr:1:26" | ||||
| 
 | ||||
|       issue.message.should eq "Use `reject(Nil)` instead of `reject {...}`" | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										214
									
								
								spec/ameba/rule/style/verbose_block_spec.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										214
									
								
								spec/ameba/rule/style/verbose_block_spec.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,214 @@ | |||
| require "../../../spec_helper" | ||||
| 
 | ||||
| module Ameba::Rule::Style | ||||
|   subject = VerboseBlock.new | ||||
| 
 | ||||
|   describe VerboseBlock do | ||||
|     it "passes if there is no potential performance improvements" do | ||||
|       source = Source.new %( | ||||
|         (1..3).any?(&.odd?) | ||||
|         (1..3).join('.', &.to_s) | ||||
|         (1..3).each_with_index { |i, idx| i * idx } | ||||
|         (1..3).map { |i| typeof(i) } | ||||
|         (1..3).map { |i| i || 0 } | ||||
|         (1..3).map { |i| :foo } | ||||
|         (1..3).map { |i| :foo.to_s.split.join('.') } | ||||
|         (1..3).map { :foo } | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "passes if the block argument is used within the body" do | ||||
|       source = Source.new %( | ||||
|         (1..3).map { |i| i * i } | ||||
|         (1..3).map { |j| j * j.to_i64 } | ||||
|         (1..3).map { |k| k.to_i64 * k } | ||||
|         (1..3).map { |l| l.to_i64 * l.to_i64 } | ||||
|         (1..3).map { |m| m.to_s[start: m.to_i64, count: 3]? } | ||||
|         (1..3).map { |n| n.to_s.split.map { |z| n.to_i * z.to_i }.join } | ||||
|       ) | ||||
|       subject.catch(source).should be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is a call with a collapsible block" do | ||||
|       source = Source.new %( | ||||
|         (1..3).any? { |i| i.odd? } | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is a call with an argument + collapsible block" do | ||||
|       source = Source.new %( | ||||
|         (1..3).join('.') { |i| i.to_s } | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     it "reports if there is a call with a collapsible block (with chained call)" do | ||||
|       source = Source.new %( | ||||
|         (1..3).map { |i| i.to_s.split.reverse.join.strip } | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|     end | ||||
| 
 | ||||
|     context "properties" do | ||||
|       it "#exclude_calls_with_block" do | ||||
|         source = Source.new %( | ||||
|           (1..3).in_groups_of(1) { |i| i.map(&.to_s) } | ||||
|         ) | ||||
|         rule = VerboseBlock.new | ||||
|         rule | ||||
|           .tap(&.exclude_calls_with_block = true) | ||||
|           .catch(source).should be_valid | ||||
|         rule | ||||
|           .tap(&.exclude_calls_with_block = false) | ||||
|           .catch(source).should_not be_valid | ||||
|       end | ||||
| 
 | ||||
|       it "#exclude_multiple_line_blocks" do | ||||
|         source = Source.new %( | ||||
|           (1..3).any? do |i| | ||||
|             i.odd? | ||||
|           end | ||||
|         ) | ||||
|         rule = VerboseBlock.new | ||||
|         rule | ||||
|           .tap(&.exclude_multiple_line_blocks = true) | ||||
|           .catch(source).should be_valid | ||||
|         rule | ||||
|           .tap(&.exclude_multiple_line_blocks = false) | ||||
|           .catch(source).should_not be_valid | ||||
|       end | ||||
| 
 | ||||
|       it "#exclude_prefix_operators" do | ||||
|         source = Source.new %( | ||||
|           (1..3).sum { |i| +i } | ||||
|           (1..3).sum { |i| -i } | ||||
|         ) | ||||
|         rule = VerboseBlock.new | ||||
|         rule | ||||
|           .tap(&.exclude_prefix_operators = true) | ||||
|           .catch(source).should be_valid | ||||
|         rule | ||||
|           .tap(&.exclude_prefix_operators = false) | ||||
|           .catch(source).should_not be_valid | ||||
|       end | ||||
| 
 | ||||
|       it "#exclude_operators" do | ||||
|         source = Source.new %( | ||||
|           (1..3).sum { |i| i * 2 } | ||||
|         ) | ||||
|         rule = VerboseBlock.new | ||||
|         rule | ||||
|           .tap(&.exclude_operators = true) | ||||
|           .catch(source).should be_valid | ||||
|         rule | ||||
|           .tap(&.exclude_operators = false) | ||||
|           .catch(source).should_not be_valid | ||||
|       end | ||||
| 
 | ||||
|       it "#exclude_setters" do | ||||
|         source = Source.new %( | ||||
|           Char::Reader.new("abc").tap { |reader| reader.pos = 0 } | ||||
|         ) | ||||
|         rule = VerboseBlock.new | ||||
|         rule | ||||
|           .tap(&.exclude_setters = true) | ||||
|           .catch(source).should be_valid | ||||
|         rule | ||||
|           .tap(&.exclude_setters = false) | ||||
|           .catch(source).should_not be_valid | ||||
|       end | ||||
| 
 | ||||
|       it "#max_line_length" do | ||||
|         source = Source.new %( | ||||
|           (1..3).tap &.tap &.tap &.tap &.tap &.tap &.tap do |i| | ||||
|             i.to_s.reverse.strip.blank? | ||||
|           end | ||||
|         ) | ||||
|         rule = VerboseBlock.new | ||||
|         rule | ||||
|           .tap(&.max_line_length = 60) | ||||
|           .catch(source).should be_valid | ||||
|         rule | ||||
|           .tap(&.max_line_length = nil) | ||||
|           .catch(source).should_not be_valid | ||||
|       end | ||||
| 
 | ||||
|       it "#max_length" do | ||||
|         source = Source.new %( | ||||
|           (1..3).tap { |i| i.to_s.split.reverse.join.strip.blank? } | ||||
|         ) | ||||
|         rule = VerboseBlock.new | ||||
|         rule | ||||
|           .tap(&.max_length = 30) | ||||
|           .catch(source).should be_valid | ||||
|         rule | ||||
|           .tap(&.max_length = nil) | ||||
|           .catch(source).should_not be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context "macro" do | ||||
|       it "reports in macro scope" do | ||||
|         source = Source.new %( | ||||
|           {{ (1..3).any? { |i| i.odd? } }} | ||||
|         ) | ||||
|         subject.catch(source).should_not be_valid | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     it "reports call args and named_args" do | ||||
|       short_block_variants = { | ||||
|         %|map(&.to_s.[start: 0.to_i64, count: 3]?)|, | ||||
|         %|map(&.to_s.[0.to_i64, count: 3]?)|, | ||||
|         %|map(&.to_s.[0.to_i64, 3]?)|, | ||||
|         %|map(&.to_s.[start: 0.to_i64, count: 3]=("foo"))|, | ||||
|         %|map(&.to_s.[0.to_i64, count: 3]=("foo"))|, | ||||
|         %|map(&.to_s.[0.to_i64, 3]=("foo"))|, | ||||
|         %|map(&.to_s.camelcase(lower: true))|, | ||||
|         %|map(&.to_s.camelcase)|, | ||||
|         %|map(&.to_s.gsub('_', '-'))|, | ||||
|         %|map(&.in?(*{1, 2, 3}, **{foo: :bar}))|, | ||||
|         %|map(&.in?(1, *foo, 3, **bar))|, | ||||
|         %|join(separator: '.', &.to_s)|, | ||||
|       } | ||||
| 
 | ||||
|       source = Source.new path: "source.cr", code: %( | ||||
|         (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3]? } | ||||
|         (1..3).map { |i| i.to_s[0.to_i64, count: 3]? } | ||||
|         (1..3).map { |i| i.to_s[0.to_i64, 3]? } | ||||
|         (1..3).map { |i| i.to_s[start: 0.to_i64, count: 3] = "foo" } | ||||
|         (1..3).map { |i| i.to_s[0.to_i64, count: 3] = "foo" } | ||||
|         (1..3).map { |i| i.to_s[0.to_i64, 3] = "foo" } | ||||
|         (1..3).map { |i| i.to_s.camelcase(lower: true) } | ||||
|         (1..3).map { |i| i.to_s.camelcase } | ||||
|         (1..3).map { |i| i.to_s.gsub('_', '-') } | ||||
|         (1..3).map { |i| i.in?(*{1, 2, 3}, **{foo: :bar}) } | ||||
|         (1..3).map { |i| i.in?(1, *foo, 3, **bar) } | ||||
|         (1..3).join(separator: '.') { |i| i.to_s } | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|       source.issues.size.should eq(short_block_variants.size) | ||||
| 
 | ||||
|       source.issues.each_with_index do |issue, i| | ||||
|         issue.message.should eq(VerboseBlock::MSG % short_block_variants[i]) | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     it "reports rule, pos and message" do | ||||
|       source = Source.new path: "source.cr", code: %( | ||||
|         (1..3).any? { |i| i.odd? } | ||||
|       ) | ||||
|       subject.catch(source).should_not be_valid | ||||
|       source.issues.size.should eq 1 | ||||
| 
 | ||||
|       issue = source.issues.first | ||||
|       issue.rule.should_not be_nil | ||||
|       issue.location.to_s.should eq "source.cr:1:8" | ||||
|       issue.end_location.to_s.should eq "source.cr:1:26" | ||||
| 
 | ||||
|       issue.message.should eq "Use short block notation instead: `any?(&.odd?)`" | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -4,9 +4,7 @@ module Ameba | |||
|   describe Severity do | ||||
|     describe "#symbol" do | ||||
|       it "returns the symbol for each severity in the enum" do | ||||
|         Severity.values.each do |severity| | ||||
|           severity.symbol.should_not be_nil | ||||
|         end | ||||
|         Severity.values.each(&.symbol.should_not(be_nil)) | ||||
|       end | ||||
| 
 | ||||
|       it "returns symbol for Error" do | ||||
|  |  | |||
|  | @ -23,6 +23,18 @@ module Ameba | |||
|       end | ||||
|     end | ||||
| 
 | ||||
|     describe "#spec?" do | ||||
|       it "returns true if the source is a spec file" do | ||||
|         s = Source.new "", "./source_spec.cr" | ||||
|         s.spec?.should be_true | ||||
|       end | ||||
| 
 | ||||
|       it "returns false if the source is not a spec file" do | ||||
|         s = Source.new "", "./source.cr" | ||||
|         s.spec?.should be_false | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     describe "#matches_path?" do | ||||
|       it "returns true if source's path is matched" do | ||||
|         s = Source.new "", "source.cr" | ||||
|  |  | |||
|  | @ -3,7 +3,7 @@ require "../src/ameba" | |||
| 
 | ||||
| module Ameba | ||||
|   # Dummy Rule which does nothing. | ||||
|   struct DummyRule < Rule::Base | ||||
|   class DummyRule < Rule::Base | ||||
|     properties do | ||||
|       description : String = "Dummy rule that does nothing." | ||||
|     end | ||||
|  | @ -35,7 +35,7 @@ module Ameba | |||
|     end | ||||
|   end | ||||
| 
 | ||||
|   struct NamedRule < Rule::Base | ||||
|   class NamedRule < Rule::Base | ||||
|     properties do | ||||
|       description "A rule with a custom name." | ||||
|     end | ||||
|  | @ -45,7 +45,7 @@ module Ameba | |||
|     end | ||||
|   end | ||||
| 
 | ||||
|   struct ErrorRule < Rule::Base | ||||
|   class ErrorRule < Rule::Base | ||||
|     properties do | ||||
|       description "Always adds an error at 1:1" | ||||
|     end | ||||
|  | @ -55,7 +55,7 @@ module Ameba | |||
|     end | ||||
|   end | ||||
| 
 | ||||
|   struct ScopeRule < Rule::Base | ||||
|   class ScopeRule < Rule::Base | ||||
|     @[YAML::Field(ignore: true)] | ||||
|     getter scopes = [] of AST::Scope | ||||
| 
 | ||||
|  | @ -68,7 +68,7 @@ module Ameba | |||
|     end | ||||
|   end | ||||
| 
 | ||||
|   struct FlowExpressionRule < Rule::Base | ||||
|   class FlowExpressionRule < Rule::Base | ||||
|     @[YAML::Field(ignore: true)] | ||||
|     getter expressions = [] of AST::FlowExpression | ||||
| 
 | ||||
|  | @ -81,7 +81,7 @@ module Ameba | |||
|     end | ||||
|   end | ||||
| 
 | ||||
|   struct RedundantControlExpressionRule < Rule::Base | ||||
|   class RedundantControlExpressionRule < Rule::Base | ||||
|     @[YAML::Field(ignore: true)] | ||||
|     getter nodes = [] of Crystal::ASTNode | ||||
| 
 | ||||
|  | @ -95,7 +95,7 @@ module Ameba | |||
|   end | ||||
| 
 | ||||
|   # A rule that always raises an error | ||||
|   struct RaiseRule < Rule::Base | ||||
|   class RaiseRule < Rule::Base | ||||
|     property should_raise = false | ||||
| 
 | ||||
|     properties do | ||||
|  |  | |||
|  | @ -20,7 +20,6 @@ require "./ameba/formatter/*" | |||
| # | ||||
| # Ameba.run config | ||||
| # ``` | ||||
| # | ||||
| module Ameba | ||||
|   extend self | ||||
| 
 | ||||
|  | @ -35,7 +34,6 @@ module Ameba | |||
|   # Ameba.run | ||||
|   # Ameba.run config | ||||
|   # ``` | ||||
|   # | ||||
|   def run(config = Config.load) | ||||
|     Runner.new(config).run | ||||
|   end | ||||
|  |  | |||
|  | @ -44,7 +44,6 @@ module Ameba::AST | |||
|     #   end | ||||
|     # end | ||||
|     # ``` | ||||
|     # | ||||
|     def in_loop? | ||||
|       @parent.loop? | ||||
|     end | ||||
|  |  | |||
|  | @ -37,8 +37,7 @@ module Ameba::AST | |||
| 
 | ||||
|     # Returns true if this node or one of the parent branchables is a loop, false otherwise. | ||||
|     def loop? | ||||
|       return true if loop?(node) | ||||
|       parent.try(&.loop?) || false | ||||
|       loop?(node) || parent.try(&.loop?) || false | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -59,8 +59,6 @@ module Ameba::AST | |||
|         end | ||||
|       when Crystal::BinaryOp | ||||
|         unreachable_nodes << current_node.right if flow_expression?(current_node.left, in_loop?) | ||||
|       else | ||||
|         # nop | ||||
|       end | ||||
| 
 | ||||
|       unreachable_nodes | ||||
|  |  | |||
|  | @ -78,7 +78,7 @@ module Ameba::AST | |||
|     # scope.find_variable("foo") | ||||
|     # ``` | ||||
|     def find_variable(name : String) | ||||
|       variables.find { |v| v.name == name } || outer_scope.try &.find_variable(name) | ||||
|       variables.find(&.name.==(name)) || outer_scope.try &.find_variable(name) | ||||
|     end | ||||
| 
 | ||||
|     # Creates a new assignment for the variable. | ||||
|  | @ -117,8 +117,8 @@ module Ameba::AST | |||
| 
 | ||||
|     # Returns true instance variable assinged in this scope. | ||||
|     def assigns_ivar?(name) | ||||
|       arguments.find { |arg| arg.name == name } && | ||||
|         ivariables.find { |var| var.name == "@#{name}" } | ||||
|       arguments.find(&.name.== name) && | ||||
|         ivariables.find(&.name.== "@#{name}") | ||||
|     end | ||||
| 
 | ||||
|     # Returns true if and only if current scope represents some | ||||
|  | @ -143,7 +143,7 @@ module Ameba::AST | |||
| 
 | ||||
|     # Returns true if current scope is a def, false if not. | ||||
|     def def? | ||||
|       node.is_a? Crystal::Def | ||||
|       node.is_a?(Crystal::Def) | ||||
|     end | ||||
| 
 | ||||
|     # Returns true if this scope is a top level scope, false if not. | ||||
|  | @ -173,7 +173,7 @@ module Ameba::AST | |||
|     # the same Crystal node as `@node`. | ||||
|     def eql?(node) | ||||
|       node == @node && | ||||
|         !node.location.nil? && | ||||
|         node.location && | ||||
|         node.location == @node.location | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -97,7 +97,6 @@ module Ameba::AST::Util | |||
|   # ``` | ||||
|   # | ||||
|   # That's because not all branches return(i.e. `else` is missing). | ||||
|   # | ||||
|   def flow_expression?(node, in_loop = false) | ||||
|     return true if flow_command? node, in_loop | ||||
| 
 | ||||
|  |  | |||
|  | @ -28,7 +28,6 @@ module Ameba::AST | |||
|     # ``` | ||||
|     # Assignment.new(node, variable, scope) | ||||
|     # ``` | ||||
|     # | ||||
|     def initialize(@node, @variable, @scope) | ||||
|       if scope = @variable.scope | ||||
|         @branch = Branch.of(@node, scope) | ||||
|  | @ -49,7 +48,7 @@ module Ameba::AST | |||
|     # a ||= 1 | ||||
|     # ``` | ||||
|     def op_assign? | ||||
|       node.is_a? Crystal::OpAssign | ||||
|       node.is_a?(Crystal::OpAssign) | ||||
|     end | ||||
| 
 | ||||
|     # Returns true if this assignment is in a branch, false if not. | ||||
|  | @ -95,7 +94,6 @@ module Ameba::AST | |||
|     #   puts(b) | ||||
|     # end | ||||
|     # ``` | ||||
|     # | ||||
|     def transformed? | ||||
|       return false unless (assign = node).is_a?(Crystal::Assign) | ||||
|       return false unless (value = assign.value).is_a?(Crystal::Call) | ||||
|  |  | |||
|  | @ -27,7 +27,6 @@ module Ameba::AST | |||
|     # ``` | ||||
|     # Variable.new(node, scope) | ||||
|     # ``` | ||||
|     # | ||||
|     def initialize(@node, @scope) | ||||
|     end | ||||
| 
 | ||||
|  | @ -45,7 +44,6 @@ module Ameba::AST | |||
|     # variable.assign(node2) | ||||
|     # variable.assignment.size # => 2 | ||||
|     # ``` | ||||
|     # | ||||
|     def assign(node, scope) | ||||
|       assignments << Assignment.new(node, self, scope) | ||||
| 
 | ||||
|  | @ -60,7 +58,7 @@ module Ameba::AST | |||
|     # variable.referenced? # => true | ||||
|     # ``` | ||||
|     def referenced? | ||||
|       references.any? | ||||
|       !references.empty? | ||||
|     end | ||||
| 
 | ||||
|     # Creates a reference to this variable in some scope. | ||||
|  | @ -69,7 +67,6 @@ module Ameba::AST | |||
|     # variable = Variable.new(node, scope) | ||||
|     # variable.reference(var_node, some_scope) | ||||
|     # ``` | ||||
|     # | ||||
|     def reference(node : Crystal::Var, scope : Scope) | ||||
|       Reference.new(node, scope).tap do |reference| | ||||
|         references << reference | ||||
|  | @ -91,8 +88,8 @@ module Ameba::AST | |||
|         next if consumed_branches.includes?(assignment.branch) | ||||
|         assignment.referenced = true | ||||
| 
 | ||||
|         break unless assignment.branch | ||||
|         consumed_branches << assignment.branch.not_nil! | ||||
|         break unless branch = assignment.branch | ||||
|         consumed_branches << branch | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  | @ -175,6 +172,10 @@ module Ameba::AST | |||
|         node.accept self | ||||
|       end | ||||
| 
 | ||||
|       def references?(node : Crystal::Var) | ||||
|         @macro_literals.any?(&.value.includes?(node.name)) | ||||
|       end | ||||
| 
 | ||||
|       def visit(node : Crystal::ASTNode) | ||||
|         true | ||||
|       end | ||||
|  | @ -203,7 +204,7 @@ module Ameba::AST | |||
|     private def update_assign_reference! | ||||
|       if @assign_before_reference.nil? && | ||||
|          references.size <= assignments.size && | ||||
|          assignments.none? { |ass| ass.op_assign? } | ||||
|          assignments.none?(&.op_assign?) | ||||
|         @assign_before_reference = assignments.find { |ass| !ass.in_branch? }.try &.node | ||||
|       end | ||||
|     end | ||||
|  |  | |||
|  | @ -15,7 +15,6 @@ module Ameba::AST | |||
|     # ``` | ||||
|     # visitor = Ameba::AST::NodeVisitor.new(rule, source) | ||||
|     # ``` | ||||
|     # | ||||
|     def initialize(@rule, @source) | ||||
|       @source.ast.accept self | ||||
|     end | ||||
|  |  | |||
|  | @ -18,7 +18,6 @@ module Ameba::AST | |||
|       if flow_expression?(node, in_loop?) | ||||
|         @rule.test @source, node, FlowExpression.new(node, in_loop?) | ||||
|       end | ||||
| 
 | ||||
|       true | ||||
|     end | ||||
| 
 | ||||
|  | @ -61,7 +60,7 @@ module Ameba::AST | |||
|     end | ||||
| 
 | ||||
|     private def in_loop? | ||||
|       @loop_stack.any? | ||||
|       !@loop_stack.empty? | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -40,7 +40,7 @@ module Ameba::AST | |||
|     @skip : Array(Crystal::ASTNode.class)? | ||||
| 
 | ||||
|     def initialize(@rule, @source, skip = nil) | ||||
|       @skip = skip.try &.map { |el| el.as(Crystal::ASTNode.class) } | ||||
|       @skip = skip.try &.map(&.as(Crystal::ASTNode.class)) | ||||
|       super @rule, @source | ||||
|     end | ||||
| 
 | ||||
|  | @ -54,7 +54,7 @@ module Ameba::AST | |||
|     {% end %} | ||||
| 
 | ||||
|     def visit(node) | ||||
|       return true unless (skip = @skip) | ||||
|       return true unless skip = @skip | ||||
|       !skip.includes?(node.class) | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -28,8 +28,6 @@ module Ameba::AST | |||
|       when Crystal::Case                then traverse_case node | ||||
|       when Crystal::BinaryOp            then traverse_binary_op node | ||||
|       when Crystal::ExceptionHandler    then traverse_exception_handler node | ||||
|       else | ||||
|         # ok | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  |  | |||
|  | @ -7,7 +7,6 @@ module Ameba::AST | |||
|     RECORD_NODE_NAME = "record" | ||||
| 
 | ||||
|     @scope_queue = [] of Scope | ||||
| 
 | ||||
|     @current_scope : Scope | ||||
| 
 | ||||
|     def initialize(@rule, @source) | ||||
|  | @ -169,7 +168,6 @@ module Ameba::AST | |||
|             variable.reference(variable.node, @current_scope).explicit = false | ||||
|           end | ||||
|         end | ||||
| 
 | ||||
|         true | ||||
|       when @current_scope.top_level? && record_macro?(node) | ||||
|         false | ||||
|  |  | |||
							
								
								
									
										28
									
								
								src/ameba/ast/visitors/top_level_nodes_visitor.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										28
									
								
								src/ameba/ast/visitors/top_level_nodes_visitor.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,28 @@ | |||
| module Ameba::AST | ||||
|   # AST Visitor that visits certain nodes at a top level, which | ||||
|   # can characterize the source (i.e. require statements, modules etc.) | ||||
|   class TopLevelNodesVisitor < Crystal::Visitor | ||||
|     getter require_nodes = [] of Crystal::Require | ||||
| 
 | ||||
|     # Creates a new instance of visitor | ||||
|     def initialize(@scope : Crystal::ASTNode) | ||||
|       @scope.accept(self) | ||||
|     end | ||||
| 
 | ||||
|     # :nodoc: | ||||
|     def visit(node : Crystal::Require) | ||||
|       require_nodes << node | ||||
|     end | ||||
| 
 | ||||
|     # If a top level node is Crystal::Expressions traverse the children. | ||||
|     def visit(node : Crystal::Expressions) | ||||
|       true | ||||
|     end | ||||
| 
 | ||||
|     # A general visit method for rest of the nodes. | ||||
|     # Returns false meaning all child nodes will not be traversed. | ||||
|     def visit(node : Crystal::ASTNode) | ||||
|       false | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -8,12 +8,21 @@ module Ameba::Cli | |||
|   def run(args = ARGV) | ||||
|     opts = parse_args args | ||||
|     config = Config.load opts.config, opts.colors? | ||||
|     config.globs = opts.globs.not_nil! if opts.globs | ||||
|     config.severity = opts.fail_level.not_nil! if opts.fail_level | ||||
| 
 | ||||
|     if globs = opts.globs | ||||
|       config.globs = globs | ||||
|     end | ||||
|     if fail_level = opts.fail_level | ||||
|       config.severity = fail_level | ||||
|     end | ||||
| 
 | ||||
|     configure_formatter(config, opts) | ||||
|     configure_rules(config, opts) | ||||
| 
 | ||||
|     if opts.rules? | ||||
|       print_rules(config) | ||||
|     end | ||||
| 
 | ||||
|     runner = Ameba.run(config) | ||||
| 
 | ||||
|     if location = opts.location_to_explain | ||||
|  | @ -34,6 +43,7 @@ module Ameba::Cli | |||
|     property except : Array(String)? | ||||
|     property location_to_explain : NamedTuple(file: String, line: Int32, column: Int32)? | ||||
|     property fail_level : Severity? | ||||
|     property? rules = false | ||||
|     property? all = false | ||||
|     property? colors = true | ||||
|     property? without_affected_code = false | ||||
|  | @ -44,13 +54,14 @@ module Ameba::Cli | |||
|       parser.banner = "Usage: ameba [options] [file1 file2 ...]" | ||||
| 
 | ||||
|       parser.on("-v", "--version", "Print version") { print_version } | ||||
|       parser.on("-h", "--help", "Show this help") { show_help parser } | ||||
|       parser.on("-h", "--help", "Show this help") { print_help(parser) } | ||||
|       parser.on("-r", "--rules", "Show all available rules") { opts.rules = true } | ||||
|       parser.on("-s", "--silent", "Disable output") { opts.formatter = :silent } | ||||
|       parser.unknown_args do |f| | ||||
|         if f.size == 1 && f.first =~ /.+:\d+:\d+/ | ||||
|           configure_explain_opts(f.first, opts) | ||||
|         else | ||||
|           opts.globs = f if f.any? | ||||
|           opts.globs = f unless f.empty? | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|  | @ -66,12 +77,12 @@ module Ameba::Cli | |||
| 
 | ||||
|       parser.on("--only RULE1,RULE2,...", | ||||
|         "Run only given rules (or groups)") do |rules| | ||||
|         opts.only = rules.split "," | ||||
|         opts.only = rules.split(',') | ||||
|       end | ||||
| 
 | ||||
|       parser.on("--except RULE1,RULE2,...", | ||||
|         "Disable the given rules (or groups)") do |rules| | ||||
|         opts.except = rules.split "," | ||||
|         opts.except = rules.split(',') | ||||
|       end | ||||
| 
 | ||||
|       parser.on("--all", "Enables all available rules") do | ||||
|  | @ -84,7 +95,8 @@ module Ameba::Cli | |||
|         opts.config = "" | ||||
|       end | ||||
| 
 | ||||
|       parser.on("--fail-level SEVERITY", "Change the level of failure to exit. Defaults to Convention") do |level| | ||||
|       parser.on("--fail-level SEVERITY", | ||||
|         "Change the level of failure to exit. Defaults to Convention") do |level| | ||||
|         opts.fail_level = Severity.parse(level) | ||||
|       end | ||||
| 
 | ||||
|  | @ -107,13 +119,13 @@ module Ameba::Cli | |||
|   end | ||||
| 
 | ||||
|   private def configure_rules(config, opts) | ||||
|     if only = opts.only | ||||
|       config.rules.map! { |r| r.enabled = false; r } | ||||
|     case | ||||
|     when only = opts.only | ||||
|       config.rules.each(&.enabled = false) | ||||
|       config.update_rules(only, enabled: true) | ||||
|     elsif opts.all? | ||||
|       config.rules.map! { |r| r.enabled = true; r } | ||||
|     when opts.all? | ||||
|       config.rules.each(&.enabled = true) | ||||
|     end | ||||
| 
 | ||||
|     config.update_rules(opts.except, enabled: false) | ||||
|   end | ||||
| 
 | ||||
|  | @ -121,7 +133,8 @@ module Ameba::Cli | |||
|     if name = opts.formatter | ||||
|       config.formatter = name | ||||
|     end | ||||
|     config.formatter.config[:without_affected_code] = opts.without_affected_code? | ||||
|     config.formatter.config[:without_affected_code] = | ||||
|       opts.without_affected_code? | ||||
|   end | ||||
| 
 | ||||
|   private def configure_explain_opts(loc, opts) | ||||
|  | @ -132,10 +145,15 @@ module Ameba::Cli | |||
|   end | ||||
| 
 | ||||
|   private def parse_explain_location(arg) | ||||
|     location = arg.split(":", remove_empty: true).map &.strip | ||||
|     location = arg.split(':', remove_empty: true).map! &.strip | ||||
|     raise ArgumentError.new unless location.size === 3 | ||||
| 
 | ||||
|     file, line, column = location | ||||
|     {file: file, line: line.to_i, column: column.to_i} | ||||
|     { | ||||
|       file:   file, | ||||
|       line:   line.to_i, | ||||
|       column: column.to_i, | ||||
|     } | ||||
|   rescue | ||||
|     raise "location should have PATH:line:column format" | ||||
|   end | ||||
|  | @ -145,8 +163,18 @@ module Ameba::Cli | |||
|     exit 0 | ||||
|   end | ||||
| 
 | ||||
|   private def show_help(parser) | ||||
|   private def print_help(parser) | ||||
|     puts parser | ||||
|     exit 0 | ||||
|   end | ||||
| 
 | ||||
|   private def print_rules(config) | ||||
|     config.rules.each do |rule| | ||||
|       puts \ | ||||
|         "#{rule.name.colorize(:white)} " \ | ||||
|         "[#{rule.severity.symbol.to_s.colorize(:green)}] - " \ | ||||
|         "#{rule.description.colorize(:dark_gray)}" | ||||
|     end | ||||
|     exit 0 | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -31,7 +31,6 @@ class Ameba::Config | |||
|     !lib | ||||
|   ) | ||||
| 
 | ||||
|   setter formatter : Formatter::BaseFormatter? | ||||
|   getter rules : Array(Rule::Base) | ||||
|   property severity = Severity::Convention | ||||
| 
 | ||||
|  | @ -66,7 +65,9 @@ class Ameba::Config | |||
|     @excluded = load_array_section(config, "Excluded") | ||||
|     @globs = load_array_section(config, "Globs", DEFAULT_GLOBS) | ||||
| 
 | ||||
|     self.formatter = load_formatter_name(config) | ||||
|     if formatter_name = load_formatter_name(config) | ||||
|       self.formatter = formatter_name | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   # Loads YAML configuration file by `path`. | ||||
|  | @ -74,7 +75,6 @@ class Ameba::Config | |||
|   # ``` | ||||
|   # config = Ameba::Config.load | ||||
|   # ``` | ||||
|   # | ||||
|   def self.load(path = PATH, colors = true) | ||||
|     Colorize.enabled = colors | ||||
|     content = File.exists?(path) ? File.read path : "{}" | ||||
|  | @ -84,7 +84,7 @@ class Ameba::Config | |||
|   end | ||||
| 
 | ||||
|   def self.formatter_names | ||||
|     AVAILABLE_FORMATTERS.keys.join("|") | ||||
|     AVAILABLE_FORMATTERS.keys.join('|') | ||||
|   end | ||||
| 
 | ||||
|   # Returns a list of sources matching globs and excluded sections. | ||||
|  | @ -96,7 +96,6 @@ class Ameba::Config | |||
|   # config.excluded = ["spec"] | ||||
|   # config.sources # => list of sources pointing to files found by the wildcards | ||||
|   # ``` | ||||
|   # | ||||
|   def sources | ||||
|     (find_files_by_globs(globs) - find_files_by_globs(excluded)) | ||||
|       .map { |path| Source.new File.read(path), path } | ||||
|  | @ -111,8 +110,8 @@ class Ameba::Config | |||
|   # config.formatter | ||||
|   # ``` | ||||
|   # | ||||
|   def formatter | ||||
|     @formatter ||= Formatter::DotFormatter.new | ||||
|   property formatter : Formatter::BaseFormatter do | ||||
|     Formatter::DotFormatter.new | ||||
|   end | ||||
| 
 | ||||
|   # Sets formatter by name. | ||||
|  | @ -121,7 +120,6 @@ class Ameba::Config | |||
|   # config = Ameba::Config.load | ||||
|   # config.formatter = :progress | ||||
|   # ``` | ||||
|   # | ||||
|   def formatter=(name : String | Symbol) | ||||
|     if f = AVAILABLE_FORMATTERS[name]? | ||||
|       @formatter = f.new | ||||
|  | @ -136,15 +134,13 @@ class Ameba::Config | |||
|   # config = Ameba::Config.load | ||||
|   # config.update_rule "MyRuleName", enabled: false | ||||
|   # ``` | ||||
|   # | ||||
|   def update_rule(name, enabled = true, excluded = nil) | ||||
|     index = @rules.index { |r| r.name == name } | ||||
|     raise ArgumentError.new("Rule `#{name}` does not exist") unless index | ||||
|     rule = @rules.find(&.name.==(name)) | ||||
|     raise ArgumentError.new("Rule `#{name}` does not exist") unless rule | ||||
| 
 | ||||
|     rule = @rules[index] | ||||
|     rule.enabled = enabled | ||||
|     rule.excluded = excluded | ||||
|     @rules[index] = rule | ||||
|     rule | ||||
|       .tap(&.enabled = enabled) | ||||
|       .tap(&.excluded = excluded) | ||||
|   end | ||||
| 
 | ||||
|   # Updates rules properties. | ||||
|  | @ -159,20 +155,22 @@ class Ameba::Config | |||
|   # ``` | ||||
|   # config.update_rules %w(Group1 Group2), enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   def update_rules(names, **args) | ||||
|   def update_rules(names, enabled = true, excluded = nil) | ||||
|     names.try &.each do |name| | ||||
|       if group = @rule_groups[name]? | ||||
|         group.each { |rule| update_rule(rule.name, **args) } | ||||
|       if rules = @rule_groups[name]? | ||||
|         rules.each do |rule| | ||||
|           rule.enabled = enabled | ||||
|           rule.excluded = excluded | ||||
|         end | ||||
|       else | ||||
|         update_rule name, **args | ||||
|         update_rule name, enabled, excluded | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   private def load_formatter_name(config) | ||||
|     name = config["Formatter"]?.try &.["Name"]? | ||||
|     name ? name.to_s : nil | ||||
|     name.try(&.to_s) | ||||
|   end | ||||
| 
 | ||||
|   private def load_array_section(config, section_name, default = [] of String) | ||||
|  | @ -269,7 +267,7 @@ class Ameba::Config | |||
|         include YAML::Serializable::Strict | ||||
| 
 | ||||
|         def self.new(config = nil) | ||||
|           if (raw = config.try &.raw).is_a? Hash | ||||
|           if (raw = config.try &.raw).is_a?(Hash) | ||||
|             yaml = raw[rule_name]?.try &.to_yaml | ||||
|           end | ||||
|           from_yaml yaml || "{}" | ||||
|  |  | |||
|  | @ -6,14 +6,15 @@ module Ameba::Formatter | |||
|   class DotFormatter < BaseFormatter | ||||
|     include Util | ||||
| 
 | ||||
|     @started_at : Time? | ||||
|     @started_at : Time::Span? | ||||
|     @mutex = Thread::Mutex.new | ||||
| 
 | ||||
|     # Reports a message when inspection is started. | ||||
|     def started(sources) | ||||
|       @started_at = Time.utc # Time.monotonic | ||||
|       @started_at = Time.monotonic | ||||
| 
 | ||||
|       output << started_message(sources.size) | ||||
|       output.puts started_message(sources.size) | ||||
|       output.puts | ||||
|     end | ||||
| 
 | ||||
|     # Reports a result of the inspection of a corresponding source. | ||||
|  | @ -35,32 +36,35 @@ module Ameba::Formatter | |||
|           next if issue.disabled? | ||||
|           next if (location = issue.location).nil? | ||||
| 
 | ||||
|           output << "#{location}\n".colorize(:cyan) | ||||
|           output << "[#{issue.rule.severity.symbol}] #{issue.rule.name}: #{issue.message}\n".colorize(:red) | ||||
|           output.puts location.colorize(:cyan) | ||||
|           output.puts \ | ||||
|             "[#{issue.rule.severity.symbol}] " \ | ||||
|             "#{issue.rule.name}: " \ | ||||
|             "#{issue.message}".colorize(:red) | ||||
| 
 | ||||
|           if show_affected_code && (code = affected_code(source, location)) | ||||
|           if show_affected_code && (code = affected_code(source, location, issue.end_location)) | ||||
|             output << code.colorize(:default) | ||||
|           end | ||||
| 
 | ||||
|           output << "\n" | ||||
|           output.puts | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       output << finished_in_message(@started_at, Time.utc) # Time.monotonic | ||||
|       output << final_message(sources, failed_sources) | ||||
|       output.puts finished_in_message(@started_at, Time.monotonic) | ||||
|       output.puts final_message(sources, failed_sources) | ||||
|     end | ||||
| 
 | ||||
|     private def started_message(size) | ||||
|       if size == 1 | ||||
|         "Inspecting 1 file.\n\n".colorize(:default) | ||||
|         "Inspecting 1 file".colorize(:default) | ||||
|       else | ||||
|         "Inspecting #{size} files.\n\n".colorize(:default) | ||||
|         "Inspecting #{size} files".colorize(:default) | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     private def finished_in_message(started, finished) | ||||
|       if started && finished | ||||
|         "Finished in #{to_human(finished - started)} \n\n".colorize(:default) | ||||
|         "Finished in #{to_human(finished - started)}".colorize(:default) | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  | @ -86,11 +90,11 @@ module Ameba::Formatter | |||
| 
 | ||||
|     private def final_message(sources, failed_sources) | ||||
|       total = sources.size | ||||
|       failures = failed_sources.map { |f| f.issues.size }.sum | ||||
|       failures = failed_sources.sum(&.issues.size) | ||||
|       color = failures == 0 ? :green : :red | ||||
|       s = failures != 1 ? "s" : "" | ||||
| 
 | ||||
|       "#{total} inspected, #{failures} failure#{s}.\n".colorize color | ||||
|       "#{total} inspected, #{failures} failure#{s}".colorize(color) | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -4,9 +4,8 @@ module Ameba::Formatter | |||
|   # A formatter that shows the detailed explanation of the issue at | ||||
|   # a specific location. | ||||
|   class ExplainFormatter | ||||
|     LINE_BREAK = "\n" | ||||
|     HEADING    = "## " | ||||
|     PREFIX     = " " | ||||
|     HEADING = "## " | ||||
|     PREFIX  = " " | ||||
| 
 | ||||
|     include Util | ||||
| 
 | ||||
|  | @ -21,36 +20,36 @@ module Ameba::Formatter | |||
|     # ExplainFormatter.new output, | ||||
|     #   {file: path, line: line_number, column: column_number} | ||||
|     # ``` | ||||
|     # | ||||
|     def initialize(@output, loc) | ||||
|       @location = Crystal::Location.new(loc[:file], loc[:line], loc[:column]) | ||||
|     def initialize(@output, location) | ||||
|       @location = Crystal::Location.new(location[:file], location[:line], location[:column]) | ||||
|     end | ||||
| 
 | ||||
|     # Reports the explainations at the *@location*. | ||||
|     def finished(sources) | ||||
|       source = sources.find { |s| s.path == @location.filename } | ||||
| 
 | ||||
|       source = sources.find(&.path.==(@location.filename)) | ||||
|       return unless source | ||||
| 
 | ||||
|       source.issues.each do |issue| | ||||
|         if (location = issue.location) && | ||||
|            location.line_number == @location.line_number && | ||||
|            location.column_number == @location.column_number | ||||
|           explain(source, issue) | ||||
|         end | ||||
|       end | ||||
|       issue = source.issues.find(&.location.==(@location)) | ||||
|       return unless issue | ||||
| 
 | ||||
|       explain(source, issue) | ||||
|     end | ||||
| 
 | ||||
|     private def explain(source, issue) | ||||
|       rule = issue.rule | ||||
| 
 | ||||
|       location, end_location = | ||||
|         issue.location, issue.end_location | ||||
| 
 | ||||
|       return unless location | ||||
| 
 | ||||
|       output_title "ISSUE INFO" | ||||
|       output_paragraph [ | ||||
|         issue.message.colorize(:red).to_s, | ||||
|         @location.to_s.colorize(:cyan).to_s, | ||||
|         location.to_s.colorize(:cyan).to_s, | ||||
|       ] | ||||
| 
 | ||||
|       if affected_code = affected_code(source, @location) | ||||
|       if affected_code = affected_code(source, location, end_location, context_lines: 3) | ||||
|         output_title "AFFECTED CODE" | ||||
|         output_paragraph affected_code | ||||
|       end | ||||
|  | @ -65,19 +64,19 @@ module Ameba::Formatter | |||
|     end | ||||
| 
 | ||||
|     private def output_title(title) | ||||
|       output << HEADING.colorize(:yellow) << title.colorize(:yellow) << LINE_BREAK | ||||
|       output << LINE_BREAK | ||||
|       output << HEADING.colorize(:yellow) << title.colorize(:yellow) << '\n' | ||||
|       output << '\n' | ||||
|     end | ||||
| 
 | ||||
|     private def output_paragraph(paragraph : String) | ||||
|       output_paragraph(paragraph.split(LINE_BREAK)) | ||||
|       output_paragraph(paragraph.lines) | ||||
|     end | ||||
| 
 | ||||
|     private def output_paragraph(paragraph : Array(String)) | ||||
|       paragraph.each do |line| | ||||
|         output << PREFIX << line << LINE_BREAK | ||||
|         output << PREFIX << line << '\n' | ||||
|       end | ||||
|       output << LINE_BREAK | ||||
|       output << '\n' | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -9,7 +9,7 @@ module Ameba::Formatter | |||
|           @mutex.synchronize do | ||||
|             output.printf "%s:%d:%d: %s: [%s] %s\n", | ||||
|               source.path, loc.line_number, loc.column_number, e.rule.severity.symbol, | ||||
|               e.rule.name, e.message.gsub("\n", " ") | ||||
|               e.rule.name, e.message.gsub('\n', " ") | ||||
|           end | ||||
|         end | ||||
|       end | ||||
|  |  | |||
|  | @ -8,19 +8,20 @@ module Ameba::Formatter | |||
| 
 | ||||
|     def finished(sources) | ||||
|       super | ||||
|       issues = sources.map(&.issues).flatten | ||||
| 
 | ||||
|       issues = sources.flat_map(&.issues) | ||||
|       unless issues.any? { |issue| !issue.disabled? } | ||||
|         @output << "No issues found. File is not generated.\n" | ||||
|         @output.puts "No issues found. File is not generated." | ||||
|         return | ||||
|       end | ||||
| 
 | ||||
|       if issues.any? { |issue| issue.syntax? } | ||||
|         @output << "Unable to generate TODO file. Please fix syntax issues.\n" | ||||
|       if issues.any?(&.syntax?) | ||||
|         @output.puts "Unable to generate TODO file. Please fix syntax issues." | ||||
|         return | ||||
|       end | ||||
| 
 | ||||
|       file = generate_todo_config issues | ||||
|       @output << "Created #{file.path}\n" | ||||
|       @output.puts "Created #{file.path}" | ||||
|       file | ||||
|     end | ||||
| 
 | ||||
|  | @ -40,7 +41,7 @@ module Ameba::Formatter | |||
|     private def rule_issues_map(issues) | ||||
|       Hash(Rule::Base, Array(Issue)).new.tap do |h| | ||||
|         issues.each do |issue| | ||||
|           next if issue.disabled? || issue.rule.is_a? Rule::Lint::Syntax | ||||
|           next if issue.disabled? || issue.rule.is_a?(Rule::Lint::Syntax) | ||||
|           (h[issue.rule] ||= Array(Issue).new) << issue | ||||
|         end | ||||
|       end | ||||
|  | @ -57,10 +58,9 @@ module Ameba::Formatter | |||
|     end | ||||
| 
 | ||||
|     private def rule_todo(rule, issues) | ||||
|       rule.excluded = | ||||
|         issues.map(&.location.try &.filename.try &.to_s) | ||||
|           .compact | ||||
|           .uniq! | ||||
|       rule.excluded = issues | ||||
|         .compact_map(&.location.try &.filename.try &.to_s) | ||||
|         .uniq! | ||||
| 
 | ||||
|       {rule.name => rule}.to_yaml | ||||
|     end | ||||
|  |  | |||
|  | @ -1,22 +1,109 @@ | |||
| module Ameba::Formatter | ||||
|   module Util | ||||
|     def affected_code(source, location, max_length = 100, placeholder = " ...", prompt = "> ") | ||||
|       line, column = location.line_number, location.column_number | ||||
|       affected_line = source.lines[line - 1]? | ||||
|     def deansify(message : String?) : String? | ||||
|       message.try &.gsub(/\x1b[^m]*m/, "").presence | ||||
|     end | ||||
| 
 | ||||
|       return if affected_line.nil? || affected_line.strip.empty? | ||||
|     def trim(str, max_length = 120, ellipsis = " ...") | ||||
|       if (str.size - ellipsis.size) > max_length | ||||
|         str = str[0, max_length] | ||||
|         if str.size > ellipsis.size | ||||
|           str = str[0...-ellipsis.size] + ellipsis | ||||
|         end | ||||
|       end | ||||
|       str | ||||
|     end | ||||
| 
 | ||||
|       if affected_line.size > max_length && column < max_length | ||||
|         affected_line = affected_line[0, max_length - placeholder.size - 1] + placeholder | ||||
|     def context(lines, lineno, context_lines = 3, remove_empty = true) | ||||
|       pre_context, post_context = %w[], %w[] | ||||
| 
 | ||||
|       lines.each_with_index do |line, i| | ||||
|         case i + 1 | ||||
|         when lineno - context_lines...lineno | ||||
|           pre_context << line | ||||
|         when lineno + 1..lineno + context_lines | ||||
|           post_context << line | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       stripped = affected_line.lstrip | ||||
|       position = column - (affected_line.size - stripped.size) + prompt.size | ||||
|       if remove_empty | ||||
|         # remove empty lines at the beginning ... | ||||
|         while pre_context.first?.try(&.blank?) | ||||
|           pre_context.shift | ||||
|         end | ||||
|         # ... and the end | ||||
|         while post_context.last?.try(&.blank?) | ||||
|           post_context.pop | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       {pre_context, post_context} | ||||
|     end | ||||
| 
 | ||||
|     def affected_code(source, location, end_location = nil, context_lines = 0, max_length = 120, ellipsis = " ...", prompt = "> ") | ||||
|       lines = source.lines | ||||
|       lineno, column = | ||||
|         location.line_number, location.column_number | ||||
| 
 | ||||
|       return unless affected_line = lines[lineno - 1]?.presence | ||||
| 
 | ||||
|       if column < max_length | ||||
|         affected_line = trim(affected_line, max_length, ellipsis) | ||||
|       end | ||||
| 
 | ||||
|       show_context = context_lines > 0 | ||||
| 
 | ||||
|       if show_context | ||||
|         pre_context, post_context = | ||||
|           context(lines, lineno, context_lines) | ||||
| 
 | ||||
|         position = prompt.size + column | ||||
|         position -= 1 | ||||
|       else | ||||
|         affected_line_size, affected_line = | ||||
|           affected_line.size, affected_line.lstrip | ||||
| 
 | ||||
|         position = column - (affected_line_size - affected_line.size) + prompt.size | ||||
|         position -= 1 | ||||
|       end | ||||
| 
 | ||||
|       String.build do |str| | ||||
|         str << prompt << stripped << "\n" | ||||
|         str << " " * (position - 1) | ||||
|         if show_context | ||||
|           pre_context.try &.each do |line| | ||||
|             line = trim(line, max_length, ellipsis) | ||||
|             str << prompt | ||||
|             str.puts(line.colorize(:dark_gray)) | ||||
|           end | ||||
|         end | ||||
| 
 | ||||
|         str << prompt | ||||
|         str.puts(affected_line.colorize(:white)) | ||||
| 
 | ||||
|         str << (" " * position) | ||||
|         str << "^".colorize(:yellow) | ||||
| 
 | ||||
|         if end_location | ||||
|           end_lineno = end_location.line_number | ||||
|           end_column = end_location.column_number | ||||
| 
 | ||||
|           if end_lineno == lineno && end_column > column | ||||
|             end_position = end_column - column | ||||
|             end_position -= 1 | ||||
| 
 | ||||
|             str << ("-" * end_position).colorize(:dark_gray) | ||||
|             str << "^".colorize(:yellow) | ||||
|           end | ||||
|         end | ||||
| 
 | ||||
|         str.puts | ||||
| 
 | ||||
|         if show_context | ||||
|           post_context.try &.each do |line| | ||||
|             line = trim(line, max_length, ellipsis) | ||||
|             str << prompt | ||||
|             str.puts(line.colorize(:dark_gray)) | ||||
|           end | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -7,12 +7,11 @@ module Ameba | |||
|     # ``` | ||||
|     # find_files_by_globs(["**/*.cr", "!lib"]) | ||||
|     # ``` | ||||
|     # | ||||
|     def find_files_by_globs(globs) | ||||
|       rejected = rejected_globs(globs) | ||||
|       selected = globs - rejected | ||||
| 
 | ||||
|       expand(selected) - expand(rejected.map! { |p| p[1..-1] }) | ||||
|       expand(selected) - expand(rejected.map!(&.[1..-1])) | ||||
|     end | ||||
| 
 | ||||
|     # Expands globs. Globs can point to files or even directories. | ||||
|  | @ -20,12 +19,11 @@ module Ameba | |||
|     # ``` | ||||
|     # expand(["spec/*.cr", "src"]) # => all files in src folder + first level specs | ||||
|     # ``` | ||||
|     # | ||||
|     def expand(globs) | ||||
|       globs.map do |glob| | ||||
|       globs.flat_map do |glob| | ||||
|         glob += "/**/*.cr" if File.directory?(glob) | ||||
|         Dir[glob] | ||||
|       end.flatten.uniq! | ||||
|       end.uniq! | ||||
|     end | ||||
| 
 | ||||
|     private def rejected_globs(globs) | ||||
|  |  | |||
|  | @ -36,9 +36,8 @@ module Ameba | |||
|     #   Time.epoch(1483859302) | ||||
|     # end | ||||
|     # ``` | ||||
|     # | ||||
|     def location_disabled?(location, rule) | ||||
|       return false if Rule::SPECIAL.includes?(rule.name) | ||||
|       return false if rule.name.in?(Rule::SPECIAL) | ||||
|       return false unless line_number = location.try &.line_number.try &.- 1 | ||||
|       return false unless line = lines[line_number]? | ||||
| 
 | ||||
|  | @ -65,7 +64,6 @@ module Ameba | |||
|     # line = "# # ameba:disable Rule1, Rule2" | ||||
|     # parse_inline_directive(line) # => nil | ||||
|     # ``` | ||||
|     # | ||||
|     def parse_inline_directive(line) | ||||
|       if directive = COMMENT_DIRECTIVE_REGEX.match(line) | ||||
|         return if commented_out?(line.gsub(directive[0], "")) | ||||
|  | @ -89,8 +87,10 @@ module Ameba | |||
| 
 | ||||
|     private def line_disabled?(line, rule) | ||||
|       return false unless directive = parse_inline_directive(line) | ||||
|       Action.parse?(directive[:action]).try(&.disable?) && | ||||
|         (directive[:rules].includes?(rule.name) || directive[:rules].includes?(rule.group)) | ||||
|       return false unless Action.parse?(directive[:action]).try(&.disable?) | ||||
| 
 | ||||
|       directive[:rules].includes?(rule.name) || | ||||
|         directive[:rules].includes?(rule.group) | ||||
|     end | ||||
| 
 | ||||
|     private def commented_out?(line) | ||||
|  |  | |||
|  | @ -1,22 +1,31 @@ | |||
| module Ameba | ||||
|   # Represents an issue reported by Ameba. | ||||
|   record Issue, | ||||
|   struct Issue | ||||
|     enum Status | ||||
|       Enabled | ||||
|       Disabled | ||||
|     end | ||||
| 
 | ||||
|     # A rule that triggers this issue. | ||||
|     rule : Rule::Base, | ||||
|     getter rule : Rule::Base | ||||
| 
 | ||||
|     # Location of the issue. | ||||
|     location : Crystal::Location?, | ||||
|     getter location : Crystal::Location? | ||||
| 
 | ||||
|     # End location of the issue. | ||||
|     end_location : Crystal::Location?, | ||||
|     getter end_location : Crystal::Location? | ||||
| 
 | ||||
|     # Issue message. | ||||
|     message : String, | ||||
|     getter message : String | ||||
| 
 | ||||
|     # Issue status. | ||||
|     status : Symbol? do | ||||
|     def disabled? | ||||
|       status == :disabled | ||||
|     getter status : Status | ||||
| 
 | ||||
|     delegate :enabled?, :disabled?, | ||||
|       to: status | ||||
| 
 | ||||
|     def initialize(@rule, @location, @end_location, @message, status : Status? = nil) | ||||
|       @status = status || Status::Enabled | ||||
|     end | ||||
| 
 | ||||
|     def syntax? | ||||
|  |  | |||
|  | @ -5,37 +5,46 @@ module Ameba | |||
|     getter issues = [] of Issue | ||||
| 
 | ||||
|     # Adds a new issue to the list of issues. | ||||
|     def add_issue(rule, location : Crystal::Location?, end_location : Crystal::Location?, message, status = nil) | ||||
|       status ||= :disabled if location_disabled?(location, rule) | ||||
|       issues << Issue.new rule, location, end_location, message, status | ||||
|     def add_issue(rule, location, end_location, message, status : Issue::Status? = nil) : Issue | ||||
|       status ||= | ||||
|         Issue::Status::Disabled if location_disabled?(location, rule) | ||||
| 
 | ||||
|       Issue.new(rule, location, end_location, message, status).tap do |issue| | ||||
|         issues << issue | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     # Adds a new issue for AST *node*. | ||||
|     def add_issue(rule, node : Crystal::ASTNode, message, **args) | ||||
|       add_issue rule, node.location, node.end_location, message, **args | ||||
|     # Adds a new issue for Crystal AST *node*. | ||||
|     def add_issue(rule, node : Crystal::ASTNode, message, status : Issue::Status? = nil) : Issue | ||||
|       add_issue rule, node.location, node.end_location, message, status | ||||
|     end | ||||
| 
 | ||||
|     # Adds a new issue for Crystal *token*. | ||||
|     def add_issue(rule, token : Crystal::Token, message, **args) | ||||
|       add_issue rule, token.location, nil, message, **args | ||||
|     def add_issue(rule, token : Crystal::Token, message, status : Issue::Status? = nil) : Issue | ||||
|       add_issue rule, token.location, nil, message, status | ||||
|     end | ||||
| 
 | ||||
|     # Adds a new issue for *location* defined by line and column numbers. | ||||
|     def add_issue(rule, location : Tuple(Int32, Int32), message, **args) | ||||
|       location = Crystal::Location.new path, *location | ||||
|       add_issue rule, location, nil, message, **args | ||||
|     def add_issue(rule, location : {Int32, Int32}, message, status : Issue::Status? = nil) : Issue | ||||
|       location = | ||||
|         Crystal::Location.new(path, *location) | ||||
| 
 | ||||
|       add_issue rule, location, nil, message, status | ||||
|     end | ||||
| 
 | ||||
|     # Adds a new issue for *location* and *end_location* defined by line and column numbers. | ||||
|     def add_issue(rule, location : Tuple(Int32, Int32), end_location : Tuple(Int32, Int32), message, **args) | ||||
|       location = Crystal::Location.new path, *location | ||||
|       end_location = Crystal::Location.new path, *end_location | ||||
|       add_issue rule, location, end_location, message, **args | ||||
|     def add_issue(rule, location : {Int32, Int32}, end_location : {Int32, Int32}, message, status : Issue::Status? = nil) : Issue | ||||
|       location = | ||||
|         Crystal::Location.new(path, *location) | ||||
|       end_location = | ||||
|         Crystal::Location.new(path, *end_location) | ||||
| 
 | ||||
|       add_issue rule, location, end_location, message, status | ||||
|     end | ||||
| 
 | ||||
|     # Returns true if the list of not disabled issues is empty, false otherwise. | ||||
|     # Returns `true` if the list of not disabled issues is empty, `false` otherwise. | ||||
|     def valid? | ||||
|       issues.reject(&.disabled?).empty? | ||||
|       issues.none?(&.enabled?) | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -10,7 +10,7 @@ module Ameba::Rule | |||
|   # inherits from this struct: | ||||
|   # | ||||
|   # ``` | ||||
|   # struct MyRule < Ameba::Rule::Base | ||||
|   # class MyRule < Ameba::Rule::Base | ||||
|   #   def test(source) | ||||
|   #     if invalid?(source) | ||||
|   #       issue_for line, column, "Something wrong." | ||||
|  | @ -26,8 +26,7 @@ module Ameba::Rule | |||
|   # Enforces rules to implement an abstract `#test` method which | ||||
|   # is designed to test the source passed in. If source has issues | ||||
|   # that are tested by this rule, it should add an issue. | ||||
|   # | ||||
|   abstract struct Base | ||||
|   abstract class Base | ||||
|     include Config::RuleConfig | ||||
| 
 | ||||
|     # This method is designed to test the source passed in. If source has issues | ||||
|  | @ -50,22 +49,20 @@ module Ameba::Rule | |||
|     # source = MyRule.new.catch(source) | ||||
|     # source.valid? | ||||
|     # ``` | ||||
|     # | ||||
|     def catch(source : Source) | ||||
|       source.tap { |s| test s } | ||||
|       source.tap { test source } | ||||
|     end | ||||
| 
 | ||||
|     # Returns a name of this rule, which is basically a class name. | ||||
|     # | ||||
|     # ``` | ||||
|     # struct MyRule < Ameba::Rule::Base | ||||
|     # class MyRule < Ameba::Rule::Base | ||||
|     #   def test(source) | ||||
|     #   end | ||||
|     # end | ||||
|     # | ||||
|     # MyRule.new.name # => "MyRule" | ||||
|     # ``` | ||||
|     # | ||||
|     def name | ||||
|       {{@type}}.rule_name | ||||
|     end | ||||
|  | @ -73,13 +70,12 @@ module Ameba::Rule | |||
|     # Returns a group this rule belong to. | ||||
|     # | ||||
|     # ``` | ||||
|     # struct MyGroup::MyRule < Ameba::Rule::Base | ||||
|     # class MyGroup::MyRule < Ameba::Rule::Base | ||||
|     #   # ... | ||||
|     # end | ||||
|     # | ||||
|     # MyGroup::MyRule.new.group # => "MyGroup" | ||||
|     # ``` | ||||
|     # | ||||
|     def group | ||||
|       {{@type}}.group_name | ||||
|     end | ||||
|  | @ -91,11 +87,10 @@ module Ameba::Rule | |||
|     # ``` | ||||
|     # my_rule.excluded?(source) # => true or false | ||||
|     # ``` | ||||
|     # | ||||
|     def excluded?(source) | ||||
|       excluded.try &.any? do |path| | ||||
|         source.matches_path?(path) || | ||||
|           Dir.glob(path).any? { |glob| source.matches_path? glob } | ||||
|           Dir.glob(path).any? { |glob| source.matches_path?(glob) } | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  | @ -105,13 +100,12 @@ module Ameba::Rule | |||
|     # ``` | ||||
|     # my_rule.special? # => true or false | ||||
|     # ``` | ||||
|     # | ||||
|     def special? | ||||
|       SPECIAL.includes? name | ||||
|       name.in?(SPECIAL) | ||||
|     end | ||||
| 
 | ||||
|     def ==(other) | ||||
|       name == other.try &.name | ||||
|       name == other.try(&.name) | ||||
|     end | ||||
| 
 | ||||
|     def hash | ||||
|  | @ -123,11 +117,11 @@ module Ameba::Rule | |||
|     end | ||||
| 
 | ||||
|     protected def self.rule_name | ||||
|       name.gsub("Ameba::Rule::", "").gsub("::", "/") | ||||
|       name.gsub("Ameba::Rule::", "").gsub("::", '/') | ||||
|     end | ||||
| 
 | ||||
|     protected def self.group_name | ||||
|       rule_name.split("/")[0...-1].join("/") | ||||
|       rule_name.split('/')[0...-1].join('/') | ||||
|     end | ||||
| 
 | ||||
|     protected def self.subclasses | ||||
|  | @ -146,7 +140,7 @@ module Ameba::Rule | |||
|     # module Ameba | ||||
|     #   # This is a test rule. | ||||
|     #   # Does nothing. | ||||
|     #   struct MyRule < Ameba::Rule::Base | ||||
|     #   class MyRule < Ameba::Rule::Base | ||||
|     #     def test(source) | ||||
|     #     end | ||||
|     #   end | ||||
|  | @ -156,8 +150,11 @@ module Ameba::Rule | |||
|     # ``` | ||||
|     def self.parsed_doc | ||||
|       source = File.read(path_to_source_file) | ||||
|       nodes = Crystal::Parser.new(source).tap(&.wants_doc = true).parse | ||||
|       type_name = rule_name.split("/").last? | ||||
|       nodes = Crystal::Parser.new(source) | ||||
|         .tap(&.wants_doc = true) | ||||
|         .parse | ||||
|       type_name = rule_name.split('/').last? | ||||
| 
 | ||||
|       DocFinder.new(nodes, type_name).doc | ||||
|     end | ||||
| 
 | ||||
|  | @ -190,7 +187,6 @@ module Ameba::Rule | |||
|   # ``` | ||||
|   # Ameba::Rule.rules # => [Rule1, Rule2, ....] | ||||
|   # ``` | ||||
|   # | ||||
|   def self.rules | ||||
|     Base.subclasses | ||||
|   end | ||||
|  |  | |||
|  | @ -8,8 +8,7 @@ module Ameba::Rule::Layout | |||
|   #   Enabled: true | ||||
|   #   MaxLength: 100 | ||||
|   # ``` | ||||
|   # | ||||
|   struct LineLength < Base | ||||
|   class LineLength < Base | ||||
|     properties do | ||||
|       enabled false | ||||
|       description "Disallows lines longer than `MaxLength` number of symbols" | ||||
|  | @ -20,9 +19,7 @@ module Ameba::Rule::Layout | |||
| 
 | ||||
|     def test(source) | ||||
|       source.lines.each_with_index do |line, index| | ||||
|         next unless line.size > max_length | ||||
| 
 | ||||
|         issue_for({index + 1, max_length + 1}, MSG) | ||||
|         issue_for({index + 1, max_length + 1}, MSG) if line.size > max_length | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -7,8 +7,7 @@ module Ameba::Rule::Layout | |||
|   # Layout/TrailingBlankLines: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct TrailingBlankLines < Base | ||||
|   class TrailingBlankLines < Base | ||||
|     properties do | ||||
|       description "Disallows trailing blank lines" | ||||
|     end | ||||
|  | @ -24,9 +23,9 @@ module Ameba::Rule::Layout | |||
|       source_lines_size = source_lines.size | ||||
|       return if source_lines_size == 1 && last_source_line.empty? | ||||
| 
 | ||||
|       last_line_not_empty = !last_source_line.empty? | ||||
|       if source_lines_size >= 1 && (source_lines.last(2).join.strip.empty? || last_line_not_empty) | ||||
|         issue_for({source_lines_size - 1, 1}, last_line_not_empty ? MSG_FINAL_NEWLINE : MSG) | ||||
|       last_line_empty = last_source_line.empty? | ||||
|       if source_lines_size >= 1 && (source_lines.last(2).join.blank? || !last_line_empty) | ||||
|         issue_for({source_lines_size - 1, 1}, last_line_empty ? MSG : MSG_FINAL_NEWLINE) | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -7,8 +7,7 @@ module Ameba::Rule::Layout | |||
|   # Layout/TrailingWhitespace: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct TrailingWhitespace < Base | ||||
|   class TrailingWhitespace < Base | ||||
|     properties do | ||||
|       description "Disallows trailing whitespaces" | ||||
|     end | ||||
|  | @ -17,8 +16,7 @@ module Ameba::Rule::Layout | |||
| 
 | ||||
|     def test(source) | ||||
|       source.lines.each_with_index do |line, index| | ||||
|         next unless line =~ /\s$/ | ||||
|         issue_for({index + 1, line.size}, MSG) | ||||
|         issue_for({index + 1, line.size}, MSG) if line =~ /\s$/ | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -17,8 +17,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/BadDirective: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct BadDirective < Base | ||||
|   class BadDirective < Base | ||||
|     properties do | ||||
|       description "Reports bad comment directives" | ||||
|     end | ||||
|  | @ -48,7 +47,9 @@ module Ameba::Rule::Lint | |||
| 
 | ||||
|     private def check_rules(source, token, rules) | ||||
|       bad_names = rules - ALL_RULE_NAMES - ALL_GROUP_NAMES | ||||
|       issue_for token, "Such rules do not exist: %s" % bad_names.join(", ") unless bad_names.empty? | ||||
|       return if bad_names.empty? | ||||
| 
 | ||||
|       issue_for token, "Such rules do not exist: %s" % bad_names.join(", ") | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -19,23 +19,21 @@ module Ameba::Rule::Lint | |||
|   # Lint/ComparisonToBoolean: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct ComparisonToBoolean < Base | ||||
|   class ComparisonToBoolean < Base | ||||
|     properties do | ||||
|       enabled false | ||||
|       description "Disallows comparison to booleans" | ||||
|     end | ||||
| 
 | ||||
|     MSG = "Comparison to a boolean is pointless" | ||||
|     MSG      = "Comparison to a boolean is pointless" | ||||
|     OP_NAMES = %w(== != ===) | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       comparison = %w(== != ===).includes?(node.name) | ||||
|       to_boolean = node.args.first?.try &.is_a?(Crystal::BoolLiteral) || | ||||
|       comparison = node.name.in?(OP_NAMES) | ||||
|       to_boolean = node.args.first?.try(&.is_a?(Crystal::BoolLiteral)) || | ||||
|                    node.obj.is_a?(Crystal::BoolLiteral) | ||||
| 
 | ||||
|       return unless comparison && to_boolean | ||||
| 
 | ||||
|       issue_for node, MSG | ||||
|       issue_for node, MSG if comparison && to_boolean | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -10,8 +10,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/DebuggerStatement: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct DebuggerStatement < Base | ||||
|   class DebuggerStatement < Base | ||||
|     properties do | ||||
|       description "Disallows calls to debugger" | ||||
|     end | ||||
|  |  | |||
							
								
								
									
										31
									
								
								src/ameba/rule/lint/duplicated_require.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										31
									
								
								src/ameba/rule/lint/duplicated_require.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,31 @@ | |||
| module Ameba::Rule::Lint | ||||
|   # A rule that reports duplicated require statements. | ||||
|   # | ||||
|   # ``` | ||||
|   # require "./thing" | ||||
|   # require "./stuff" | ||||
|   # require "./thing" # duplicated require | ||||
|   # ``` | ||||
|   # | ||||
|   # YAML configuration example: | ||||
|   # | ||||
|   # ``` | ||||
|   # Lint/DuplicatedRequire: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   class DuplicatedRequire < Base | ||||
|     properties do | ||||
|       description "Reports duplicated require statements" | ||||
|     end | ||||
| 
 | ||||
|     MSG = "Duplicated require of `%s`" | ||||
| 
 | ||||
|     def test(source) | ||||
|       nodes = AST::TopLevelNodesVisitor.new(source.ast).require_nodes | ||||
|       nodes.each_with_object([] of String) do |node, processed_require_strings| | ||||
|         issue_for(node, MSG % node.string) if processed_require_strings.includes?(node.string) | ||||
|         processed_require_strings << node.string | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -38,8 +38,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/EmptyEnsure | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct EmptyEnsure < Base | ||||
|   class EmptyEnsure < Base | ||||
|     properties do | ||||
|       description "Disallows empty ensure statement" | ||||
|     end | ||||
|  |  | |||
|  | @ -27,13 +27,12 @@ module Ameba::Rule::Lint | |||
|   # Lint/EmptyExpression: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct EmptyExpression < Base | ||||
|   class EmptyExpression < Base | ||||
|     include AST::Util | ||||
| 
 | ||||
|     properties do | ||||
|       description "Disallows empty expressions" | ||||
|       enabled false | ||||
|       description "Disallows empty expressions" | ||||
|     end | ||||
| 
 | ||||
|     MSG      = "Avoid empty expression %s" | ||||
|  | @ -41,8 +40,7 @@ module Ameba::Rule::Lint | |||
| 
 | ||||
|     def test(source, node : Crystal::NilLiteral) | ||||
|       exp = node_source(node, source.lines).try &.join | ||||
| 
 | ||||
|       return if exp.nil? || exp == "nil" | ||||
|       return if exp.in?(nil, "nil") | ||||
| 
 | ||||
|       issue_for node, MSG % exp | ||||
|     end | ||||
|  |  | |||
|  | @ -37,7 +37,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/EmptyLoop: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   struct EmptyLoop < Base | ||||
|   class EmptyLoop < Base | ||||
|     include AST::Util | ||||
| 
 | ||||
|     properties do | ||||
|  |  | |||
|  | @ -19,8 +19,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/HashDuplicatedKey: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct HashDuplicatedKey < Base | ||||
|   class HashDuplicatedKey < Base | ||||
|     properties do | ||||
|       description "Disallows duplicated keys in hash literals" | ||||
|     end | ||||
|  | @ -28,7 +27,7 @@ module Ameba::Rule::Lint | |||
|     MSG = "Duplicated keys in hash literal: %s" | ||||
| 
 | ||||
|     def test(source, node : Crystal::HashLiteral) | ||||
|       return unless (keys = duplicated_keys(node.entries)).any? | ||||
|       return if (keys = duplicated_keys(node.entries)).empty? | ||||
| 
 | ||||
|       issue_for node, MSG % keys.join(", ") | ||||
|     end | ||||
|  | @ -36,7 +35,7 @@ module Ameba::Rule::Lint | |||
|     private def duplicated_keys(entries) | ||||
|       entries.map(&.key) | ||||
|         .group_by(&.itself) | ||||
|         .select { |_, v| v.size > 1 } | ||||
|         .tap(&.select! { |_, v| v.size > 1 }) | ||||
|         .map { |k, _| k } | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -19,8 +19,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/LiteralInCondition: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct LiteralInCondition < Base | ||||
|   class LiteralInCondition < Base | ||||
|     include AST::Util | ||||
| 
 | ||||
|     properties do | ||||
|  | @ -31,8 +30,7 @@ module Ameba::Rule::Lint | |||
|     MSG = "Literal value found in conditional" | ||||
| 
 | ||||
|     def check_node(source, node) | ||||
|       return unless literal?(node.cond) | ||||
|       issue_for node, MSG | ||||
|       issue_for node, MSG if literal?(node.cond) | ||||
|     end | ||||
| 
 | ||||
|     def test(source, node : Crystal::If) | ||||
|  |  | |||
|  | @ -15,8 +15,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/LiteralInInterpolation | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct LiteralInInterpolation < Base | ||||
|   class LiteralInInterpolation < Base | ||||
|     include AST::Util | ||||
| 
 | ||||
|     properties do | ||||
|  |  | |||
|  | @ -23,12 +23,12 @@ module Ameba::Rule::Lint | |||
|   #   StringArrayUnwantedSymbols: ',"' | ||||
|   #   SymbolArrayUnwantedSymbols: ',:' | ||||
|   # ``` | ||||
|   # | ||||
|   struct PercentArrays < Base | ||||
|   class PercentArrays < Base | ||||
|     properties do | ||||
|       description "Disallows some unwanted symbols in percent array literals" | ||||
|       string_array_unwanted_symbols ",\"" | ||||
|       symbol_array_unwanted_symbols ",:" | ||||
| 
 | ||||
|       string_array_unwanted_symbols %(,") | ||||
|       symbol_array_unwanted_symbols %(,:) | ||||
|     end | ||||
| 
 | ||||
|     MSG = "Symbols `%s` may be unwanted in %s array literals" | ||||
|  | @ -49,8 +49,6 @@ module Ameba::Rule::Lint | |||
|             issue_for start_token.not_nil!, issue.not_nil! | ||||
|           end | ||||
|           issue = start_token = nil | ||||
|         else | ||||
|           # nop | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|  | @ -61,14 +59,11 @@ module Ameba::Rule::Lint | |||
|         check_array_entry entry, string_array_unwanted_symbols, "%w" | ||||
|       when .starts_with? "%i" | ||||
|         check_array_entry entry, symbol_array_unwanted_symbols, "%i" | ||||
|       else | ||||
|         # nop | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     private def check_array_entry(entry, symbols, literal) | ||||
|       return unless entry =~ /[#{symbols}]/ | ||||
|       MSG % {symbols, literal} | ||||
|       MSG % {symbols, literal} if entry =~ /[#{symbols}]/ | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -22,8 +22,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/RandZero: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct RandZero < Base | ||||
|   class RandZero < Base | ||||
|     properties do | ||||
|       description "Disallows rand zero calls" | ||||
|     end | ||||
|  | @ -34,9 +33,9 @@ module Ameba::Rule::Lint | |||
|       return unless node.name == "rand" && | ||||
|                     node.args.size == 1 && | ||||
|                     (arg = node.args.first) && | ||||
|                     (arg.is_a? Crystal::NumberLiteral) && | ||||
|                     arg.is_a?(Crystal::NumberLiteral) && | ||||
|                     (value = arg.value) && | ||||
|                     (value == "0" || value == "1") | ||||
|                     value.in?("0", "1") | ||||
| 
 | ||||
|       issue_for node, MSG % node | ||||
|     end | ||||
|  |  | |||
|  | @ -20,8 +20,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/RedundantStringCoersion | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct RedundantStringCoercion < Base | ||||
|   class RedundantStringCoercion < Base | ||||
|     include AST::Util | ||||
| 
 | ||||
|     properties do | ||||
|  | @ -31,7 +30,9 @@ module Ameba::Rule::Lint | |||
|     MSG = "Redundant use of `Object#to_s` in interpolation" | ||||
| 
 | ||||
|     def test(source, node : Crystal::StringInterpolation) | ||||
|       string_coercion_nodes(node).each { |n| issue_for n.name_location, n.end_location, MSG } | ||||
|       string_coercion_nodes(node).each do |n| | ||||
|         issue_for n.name_location, n.end_location, MSG | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     private def string_coercion_nodes(node) | ||||
|  |  | |||
|  | @ -26,8 +26,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/RedundantWithIndex: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct RedundantWithIndex < Base | ||||
|   class RedundantWithIndex < Base | ||||
|     properties do | ||||
|       description "Disallows redundant `with_index` calls" | ||||
|     end | ||||
|  | @ -35,15 +34,14 @@ module Ameba::Rule::Lint | |||
|     def test(source, node : Crystal::Call) | ||||
|       args, block = node.args, node.block | ||||
| 
 | ||||
|       return if args.size > 1 || block.nil? || with_index_arg?(block.not_nil!) | ||||
|       return if block.nil? || args.size > 1 | ||||
|       return if with_index_arg?(block) | ||||
| 
 | ||||
|       case node.name | ||||
|       when "with_index" | ||||
|         report source, node, "Remove redundant with_index" | ||||
|       when "each_with_index" | ||||
|         report source, node, "Use each instead of each_with_index" | ||||
|       else | ||||
|         # nop | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  |  | |||
|  | @ -27,21 +27,20 @@ module Ameba::Rule::Lint | |||
|   # Lint/RedundantWithObject: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct RedundantWithObject < Base | ||||
|   class RedundantWithObject < Base | ||||
|     properties do | ||||
|       description "Disallows redundant `with_object` calls" | ||||
|     end | ||||
| 
 | ||||
|     MSG = "Use `each` instead of `each_with_object`" | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return if node.name != "each_with_object" || | ||||
|                 node.args.size != 1 || | ||||
|                 node.block.nil? || | ||||
|                 with_index_arg?(node.block.not_nil!) | ||||
| 
 | ||||
|       issue_for node.name_location, | ||||
|         node.name_end_location, | ||||
|         "Use each instead of each_with_object" | ||||
|       issue_for node.name_location, node.name_end_location, MSG | ||||
|     end | ||||
| 
 | ||||
|     private def with_index_arg?(block : Crystal::Block) | ||||
|  |  | |||
|  | @ -35,8 +35,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/ShadowedArgument: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct ShadowedArgument < Base | ||||
|   class ShadowedArgument < Base | ||||
|     properties do | ||||
|       description "Disallows shadowed arguments" | ||||
|     end | ||||
|  |  | |||
|  | @ -33,8 +33,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/ShadowedException: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct ShadowedException < Base | ||||
|   class ShadowedException < Base | ||||
|     properties do | ||||
|       description "Disallows rescued exception that get shadowed" | ||||
|     end | ||||
|  | @ -42,18 +41,17 @@ module Ameba::Rule::Lint | |||
|     MSG = "Exception handler has shadowed exceptions: %s" | ||||
| 
 | ||||
|     def test(source, node : Crystal::ExceptionHandler) | ||||
|       return unless excs = node.rescues | ||||
|       return unless excs = node.rescues.try &.map(&.types) | ||||
|       return if (excs = shadowed excs).empty? | ||||
| 
 | ||||
|       if (excs = shadowed excs.map(&.types)).any? | ||||
|         issue_for node, MSG % excs.join(", ") | ||||
|       end | ||||
|       issue_for node, MSG % excs.join(", ") | ||||
|     end | ||||
| 
 | ||||
|     private def shadowed(exceptions, exception_found = false) | ||||
|       previous_exceptions = [] of String | ||||
| 
 | ||||
|       exceptions.reduce([] of String) do |shadowed, excs| | ||||
|         excs = excs ? excs.map(&.to_s) : ["Exception"] | ||||
|         excs = excs.try(&.map(&.to_s)) || %w[Exception] | ||||
| 
 | ||||
|         if exception_found | ||||
|           shadowed.concat excs | ||||
|  |  | |||
|  | @ -30,8 +30,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/ShadowingOuterLocalVar: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct ShadowingOuterLocalVar < Base | ||||
|   class ShadowingOuterLocalVar < Base | ||||
|     properties do | ||||
|       description "Disallows the usage of the same name as outer local variables" \ | ||||
|                   " for block or proc arguments." | ||||
|  |  | |||
|  | @ -49,8 +49,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/SharedVarInFiber: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct SharedVarInFiber < Base | ||||
|   class SharedVarInFiber < Base | ||||
|     properties do | ||||
|       description "Disallows shared variables in fibers." | ||||
|     end | ||||
|  | @ -77,7 +76,7 @@ module Ameba::Rule::Lint | |||
|       declared_in = variable.assignments.first?.try &.branch | ||||
| 
 | ||||
|       variable.assignments | ||||
|         .reject { |assign| assign.scope.spawn_block? } | ||||
|         .reject(&.scope.spawn_block?) | ||||
|         .any? do |assign| | ||||
|           assign.branch.try(&.in_loop?) && assign.branch != declared_in | ||||
|         end | ||||
|  |  | |||
							
								
								
									
										70
									
								
								src/ameba/rule/lint/spec_focus.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										70
									
								
								src/ameba/rule/lint/spec_focus.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,70 @@ | |||
| module Ameba::Rule::Lint | ||||
|   # Checks if specs are focused. | ||||
|   # | ||||
|   # In specs `focus: true` is mainly used to focus on a spec | ||||
|   # item locally during development. However, if such change | ||||
|   # is committed, it silently runs only focused spec on all | ||||
|   # other enviroment, which is undesired. | ||||
|   # | ||||
|   # This is considered bad: | ||||
|   # | ||||
|   # ``` | ||||
|   # describe MyClass, focus: true do | ||||
|   # end | ||||
|   # | ||||
|   # describe ".new", focus: true do | ||||
|   # end | ||||
|   # | ||||
|   # context "my context", focus: true do | ||||
|   # end | ||||
|   # | ||||
|   # it "works", focus: true do | ||||
|   # end | ||||
|   # ``` | ||||
|   # | ||||
|   # And it should be written as the following: | ||||
|   # | ||||
|   # ``` | ||||
|   # describe MyClass do | ||||
|   # end | ||||
|   # | ||||
|   # describe ".new" do | ||||
|   # end | ||||
|   # | ||||
|   # context "my context" do | ||||
|   # end | ||||
|   # | ||||
|   # it "works" do | ||||
|   # end | ||||
|   # ``` | ||||
|   # | ||||
|   # YAML configuration example: | ||||
|   # | ||||
|   # ``` | ||||
|   # Lint/SpecFocus: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   class SpecFocus < Base | ||||
|     properties do | ||||
|       description "Reports focused spec items" | ||||
|     end | ||||
| 
 | ||||
|     MSG             = "Focused spec item detected" | ||||
|     SPEC_ITEM_NAMES = %w(describe context it pending) | ||||
| 
 | ||||
|     def test(source) | ||||
|       return unless source.spec? | ||||
| 
 | ||||
|       AST::NodeVisitor.new self, source | ||||
|     end | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless node.name.in?(SPEC_ITEM_NAMES) | ||||
|       return unless node.block | ||||
| 
 | ||||
|       arg = node.named_args.try &.find(&.name.== "focus") | ||||
| 
 | ||||
|       issue_for arg, MSG if arg | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -18,8 +18,7 @@ module Ameba::Rule::Lint | |||
|   # rescue e : Exception | ||||
|   # end | ||||
|   # ``` | ||||
|   # | ||||
|   struct Syntax < Base | ||||
|   class Syntax < Base | ||||
|     properties do | ||||
|       description "Reports invalid Crystal syntax" | ||||
|       severity Ameba::Severity::Error | ||||
|  |  | |||
|  | @ -24,8 +24,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/UnneededDisableDirective | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct UnneededDisableDirective < Base | ||||
|   class UnneededDisableDirective < Base | ||||
|     properties do | ||||
|       description "Reports unneeded disable directives in comments" | ||||
|     end | ||||
|  | @ -37,7 +36,7 @@ module Ameba::Rule::Lint | |||
|         next unless token.type == :COMMENT | ||||
|         next unless directive = source.parse_inline_directive(token.value.to_s) | ||||
|         next unless names = unneeded_disables(source, directive, token.location) | ||||
|         next unless names.any? | ||||
|         next if names.empty? | ||||
| 
 | ||||
|         issue_for token, MSG % names.join(", ") | ||||
|       end | ||||
|  | @ -47,11 +46,12 @@ module Ameba::Rule::Lint | |||
|       return unless directive[:action] == "disable" | ||||
| 
 | ||||
|       directive[:rules].reject do |rule_name| | ||||
|         next if rule_name == self.name | ||||
|         source.issues.any? do |issue| | ||||
|           issue.rule.name == rule_name && | ||||
|             issue.disabled? && | ||||
|             issue_at_location?(source, issue, location) | ||||
|         end && rule_name != self.name | ||||
|         end | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  |  | |||
|  | @ -41,8 +41,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/UnreachableCode: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct UnreachableCode < Base | ||||
|   class UnreachableCode < Base | ||||
|     include AST::Util | ||||
| 
 | ||||
|     properties do | ||||
|  |  | |||
|  | @ -24,8 +24,7 @@ module Ameba::Rule::Lint | |||
|   #   IgnoreBlocks: false | ||||
|   #   IgnoreProcs: false | ||||
|   # ``` | ||||
|   # | ||||
|   struct UnusedArgument < Base | ||||
|   class UnusedArgument < Base | ||||
|     properties do | ||||
|       description "Disallows unused arguments" | ||||
| 
 | ||||
|  |  | |||
|  | @ -25,8 +25,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/UselessAssign: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct UselessAssign < Base | ||||
|   class UselessAssign < Base | ||||
|     properties do | ||||
|       description "Disallows useless variable assignments" | ||||
|     end | ||||
|  |  | |||
|  | @ -30,8 +30,7 @@ module Ameba::Rule::Lint | |||
|   # Lint/UselessConditionInWhen: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct UselessConditionInWhen < Base | ||||
|   class UselessConditionInWhen < Base | ||||
|     properties do | ||||
|       description "Disallows useless conditions in when" | ||||
|     end | ||||
|  | @ -43,10 +42,7 @@ module Ameba::Rule::Lint | |||
|     # simple implementation in future. | ||||
|     protected def check_node(source, when_node, cond) | ||||
|       cond_s = cond.to_s | ||||
|       return if when_node | ||||
|                   .conds | ||||
|                   .map(&.to_s) | ||||
|                   .none? { |c| c == cond_s } | ||||
|       return if when_node.conds.none?(&.to_s.==(cond_s)) | ||||
| 
 | ||||
|       issue_for cond, MSG | ||||
|     end | ||||
|  |  | |||
|  | @ -8,8 +8,7 @@ module Ameba::Rule::Metrics | |||
|   #   Enabled: true | ||||
|   #   MaxComplexity: 10 | ||||
|   # ``` | ||||
|   # | ||||
|   struct CyclomaticComplexity < Base | ||||
|   class CyclomaticComplexity < Base | ||||
|     properties do | ||||
|       description "Disallows methods with a cyclomatic complexity higher than `MaxComplexity`" | ||||
|       max_complexity 10 | ||||
|  | @ -21,17 +20,17 @@ module Ameba::Rule::Metrics | |||
|       complexity = AST::CountingVisitor.new(node).count | ||||
| 
 | ||||
|       if complexity > max_complexity && (location = node.name_location) | ||||
|         issue_for( | ||||
|           location, | ||||
|           def_name_end_location(node), | ||||
|         issue_for location, def_name_end_location(node), | ||||
|           MSG % {complexity, max_complexity} | ||||
|         ) | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     private def def_name_end_location(node) | ||||
|       return unless location = node.name_location | ||||
|       line_number, column_number = location.line_number, location.column_number | ||||
| 
 | ||||
|       line_number, column_number = | ||||
|         location.line_number, location.column_number | ||||
| 
 | ||||
|       Crystal::Location.new(location.filename, line_number, column_number + node.name.size) | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -24,23 +24,21 @@ module Ameba::Rule::Performance | |||
|   #     - select | ||||
|   #     - reject | ||||
|   # ``` | ||||
|   # | ||||
|   struct AnyAfterFilter < Base | ||||
|     ANY_NAME = "any?" | ||||
|     MSG      = "Use `#{ANY_NAME} {...}` instead of `%s {...}.#{ANY_NAME}`" | ||||
| 
 | ||||
|   class AnyAfterFilter < Base | ||||
|     properties do | ||||
|       filter_names : Array(String) = %w(select reject) | ||||
|       description "Identifies usage of `any?` calls that follow filters." | ||||
|       filter_names : Array(String) = %w(select reject) | ||||
|     end | ||||
| 
 | ||||
|     ANY_NAME = "any?" | ||||
|     MSG      = "Use `any? {...}` instead of `%s {...}.any?`" | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless node.name == ANY_NAME && (obj = node.obj) | ||||
|       return unless obj.is_a?(Crystal::Call) && obj.block && node.block.nil? | ||||
|       return unless obj.name.in?(filter_names) | ||||
| 
 | ||||
|       if node.block.nil? && obj.is_a?(Crystal::Call) && | ||||
|          filter_names.includes?(obj.name) && !obj.block.nil? | ||||
|         issue_for obj.name_location, node.name_end_location, MSG % obj.name | ||||
|       end | ||||
|       issue_for obj.name_location, node.name_end_location, MSG % obj.name | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
							
								
								
									
										44
									
								
								src/ameba/rule/performance/any_instead_of_empty.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										44
									
								
								src/ameba/rule/performance/any_instead_of_empty.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,44 @@ | |||
| module Ameba::Rule::Performance | ||||
|   # This rule is used to identify usage of arg-less `Enumerable#any?` calls. | ||||
|   # | ||||
|   # Using `Enumerable#any?` instead of `Enumerable#empty?` might lead to an | ||||
|   # unexpected results (like `[nil, false].any? # => false`). In some cases | ||||
|   # it also might be less efficient, since it iterates until the block will | ||||
|   # return a _truthy_ value, instead of just checking if there's at least | ||||
|   # one value present. | ||||
|   # | ||||
|   # For example, this is considered invalid: | ||||
|   # | ||||
|   # ``` | ||||
|   # [1, 2, 3].any? | ||||
|   # ``` | ||||
|   # | ||||
|   # And it should be written as this: | ||||
|   # | ||||
|   # ``` | ||||
|   # ![1, 2, 3].empty? | ||||
|   # ``` | ||||
|   # | ||||
|   # YAML configuration example: | ||||
|   # | ||||
|   # ``` | ||||
|   # Performance/AnyInsteadOfEmpty: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   class AnyInsteadOfEmpty < Base | ||||
|     properties do | ||||
|       description "Identifies usage of arg-less `any?` calls." | ||||
|     end | ||||
| 
 | ||||
|     ANY_NAME = "any?" | ||||
|     MSG      = "Use `!{...}.empty?` instead of `{...}.any?`" | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless node.name == ANY_NAME | ||||
|       return unless node.block.nil? && node.args.empty? | ||||
|       return unless node.obj | ||||
| 
 | ||||
|       issue_for node.name_location, node.name_end_location, MSG | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										75
									
								
								src/ameba/rule/performance/chained_call_with_no_bang.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										75
									
								
								src/ameba/rule/performance/chained_call_with_no_bang.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,75 @@ | |||
| module Ameba::Rule::Performance | ||||
|   # This rule is used to identify usage of chained calls not utilizing | ||||
|   # the bang method variants. | ||||
|   # | ||||
|   # For example, this is considered inefficient: | ||||
|   # | ||||
|   # ``` | ||||
|   # names = %w[Alice Bob] | ||||
|   # chars = names | ||||
|   #   .flat_map(&.chars) | ||||
|   #   .uniq | ||||
|   #   .sort | ||||
|   # ``` | ||||
|   # | ||||
|   # And can be written as this: | ||||
|   # | ||||
|   # ``` | ||||
|   # names = %w[Alice Bob] | ||||
|   # chars = names | ||||
|   #   .flat_map(&.chars) | ||||
|   #   .uniq! | ||||
|   #   .sort! | ||||
|   # ``` | ||||
|   # | ||||
|   # YAML configuration example: | ||||
|   # | ||||
|   # ``` | ||||
|   # Performance/ChainedCallWithNoBang: | ||||
|   #   Enabled: true | ||||
|   #   CallNames: | ||||
|   #     - uniq | ||||
|   #     - sort | ||||
|   #     - sort_by | ||||
|   #     - shuffle | ||||
|   #     - reverse | ||||
|   # ``` | ||||
|   class ChainedCallWithNoBang < Base | ||||
|     properties do | ||||
|       description "Identifies usage of chained calls not utilizing the bang method variants." | ||||
| 
 | ||||
|       # All of those have bang method variants returning `self` | ||||
|       # and are not modifying the receiver type (like `compact` does), | ||||
|       # thus are safe to switch to the bang variant. | ||||
|       call_names : Array(String) = %w(uniq sort sort_by shuffle reverse) | ||||
|     end | ||||
| 
 | ||||
|     # All these methods are allocating a new object | ||||
|     ALLOCATING_METHOD_NAMES = %w( | ||||
|       keys values values_at map map_with_index flat_map compact_map | ||||
|       flatten compact select reject sample group_by chunks tally merge | ||||
|       combinations repeated_combinations permutations repeated_permutations | ||||
|       transpose invert chars captures named_captures clone | ||||
|     ) | ||||
| 
 | ||||
|     MSG = "Use bang method variant `%s!` after chained `%s` call" | ||||
| 
 | ||||
|     def test(source) | ||||
|       AST::NodeVisitor.new self, source, skip: [ | ||||
|         Crystal::Macro, | ||||
|         Crystal::MacroExpression, | ||||
|         Crystal::MacroIf, | ||||
|         Crystal::MacroFor, | ||||
|       ] | ||||
|     end | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless (obj = node.obj).is_a?(Crystal::Call) | ||||
|       return unless node.name.in?(call_names) | ||||
|       return unless obj.name.in?(call_names) || obj.name.in?(ALLOCATING_METHOD_NAMES) | ||||
| 
 | ||||
|       issue_for node.name_location, node.name_end_location, | ||||
|         MSG % {node.name, obj.name} | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										48
									
								
								src/ameba/rule/performance/compact_after_map.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										48
									
								
								src/ameba/rule/performance/compact_after_map.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,48 @@ | |||
| module Ameba::Rule::Performance | ||||
|   # This rule is used to identify usage of `compact` calls that follow `map`. | ||||
|   # | ||||
|   # For example, this is considered inefficient: | ||||
|   # | ||||
|   # ``` | ||||
|   # %w[Alice Bob].map(&.match(/^A./)).compact | ||||
|   # ``` | ||||
|   # | ||||
|   # And can be written as this: | ||||
|   # | ||||
|   # ``` | ||||
|   # %w[Alice Bob].compact_map(&.match(/^A./)) | ||||
|   # ``` | ||||
|   # | ||||
|   # YAML configuration example: | ||||
|   # | ||||
|   # ``` | ||||
|   # Performance/CompactAfterMap: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   class CompactAfterMap < Base | ||||
|     properties do | ||||
|       description "Identifies usage of `compact` calls that follow `map`." | ||||
|     end | ||||
| 
 | ||||
|     COMPACT_NAME = "compact" | ||||
|     MAP_NAME     = "map" | ||||
|     MSG          = "Use `compact_map {...}` instead of `map {...}.compact`" | ||||
| 
 | ||||
|     def test(source) | ||||
|       AST::NodeVisitor.new self, source, skip: [ | ||||
|         Crystal::Macro, | ||||
|         Crystal::MacroExpression, | ||||
|         Crystal::MacroIf, | ||||
|         Crystal::MacroFor, | ||||
|       ] | ||||
|     end | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless node.name == COMPACT_NAME && (obj = node.obj) | ||||
|       return unless obj.is_a?(Crystal::Call) && obj.block | ||||
|       return unless obj.name == MAP_NAME | ||||
| 
 | ||||
|       issue_for obj.name_location, node.name_end_location, MSG | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -23,17 +23,16 @@ module Ameba::Rule::Performance | |||
|   #   FilterNames: | ||||
|   #     - select | ||||
|   # ``` | ||||
|   # | ||||
|   struct FirstLastAfterFilter < Base | ||||
|   class FirstLastAfterFilter < Base | ||||
|     properties do | ||||
|       description "Identifies usage of `first/last/first?/last?` calls that follow filters." | ||||
|       filter_names : Array(String) = %w(select) | ||||
|     end | ||||
| 
 | ||||
|     CALL_NAMES  = %w(first last first? last?) | ||||
|     MSG         = "Use `find {...}` instead of `%s {...}.%s`" | ||||
|     MSG_REVERSE = "Use `reverse_each.find {...}` instead of `%s {...}.%s`" | ||||
| 
 | ||||
|     properties do | ||||
|       filter_names : Array(String) = %w(select) | ||||
|       description "Identifies usage of `first/last/first?/last?` calls that follow filters." | ||||
|     end | ||||
| 
 | ||||
|     def test(source) | ||||
|       AST::NodeVisitor.new self, source, skip: [ | ||||
|         Crystal::Macro, | ||||
|  | @ -44,14 +43,13 @@ module Ameba::Rule::Performance | |||
|     end | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless CALL_NAMES.includes?(node.name) && (obj = node.obj) | ||||
|       return if node.args.any? | ||||
|       return unless node.name.in?(CALL_NAMES) && (obj = node.obj) | ||||
|       return unless obj.is_a?(Crystal::Call) && obj.block | ||||
|       return unless node.block.nil? && node.args.empty? | ||||
|       return unless obj.name.in?(filter_names) | ||||
| 
 | ||||
|       if node.block.nil? && obj.is_a?(Crystal::Call) && | ||||
|          filter_names.includes?(obj.name) && !obj.block.nil? | ||||
|         message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE | ||||
|         issue_for obj.name_location, node.name_end_location, message % {obj.name, node.name} | ||||
|       end | ||||
|       message = node.name.includes?(CALL_NAMES.first) ? MSG : MSG_REVERSE | ||||
|       issue_for obj.name_location, node.name_end_location, message % {obj.name, node.name} | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
							
								
								
									
										48
									
								
								src/ameba/rule/performance/flatten_after_map.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										48
									
								
								src/ameba/rule/performance/flatten_after_map.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,48 @@ | |||
| module Ameba::Rule::Performance | ||||
|   # This rule is used to identify usage of `flatten` calls that follow `map`. | ||||
|   # | ||||
|   # For example, this is considered inefficient: | ||||
|   # | ||||
|   # ``` | ||||
|   # %w[Alice Bob].map(&.chars).flatten | ||||
|   # ``` | ||||
|   # | ||||
|   # And can be written as this: | ||||
|   # | ||||
|   # ``` | ||||
|   # %w[Alice Bob].flat_map(&.chars) | ||||
|   # ``` | ||||
|   # | ||||
|   # YAML configuration example: | ||||
|   # | ||||
|   # ``` | ||||
|   # Performance/FlattenAfterMap: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   class FlattenAfterMap < Base | ||||
|     properties do | ||||
|       description "Identifies usage of `flatten` calls that follow `map`." | ||||
|     end | ||||
| 
 | ||||
|     FLATTEN_NAME = "flatten" | ||||
|     MAP_NAME     = "map" | ||||
|     MSG          = "Use `flat_map {...}` instead of `map {...}.flatten`" | ||||
| 
 | ||||
|     def test(source) | ||||
|       AST::NodeVisitor.new self, source, skip: [ | ||||
|         Crystal::Macro, | ||||
|         Crystal::MacroExpression, | ||||
|         Crystal::MacroIf, | ||||
|         Crystal::MacroFor, | ||||
|       ] | ||||
|     end | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless node.name == FLATTEN_NAME && (obj = node.obj) | ||||
|       return unless obj.is_a?(Crystal::Call) && obj.block | ||||
|       return unless obj.name == MAP_NAME | ||||
| 
 | ||||
|       issue_for obj.name_location, node.name_end_location, MSG | ||||
|     end | ||||
|   end | ||||
| end | ||||
							
								
								
									
										52
									
								
								src/ameba/rule/performance/map_instead_of_block.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										52
									
								
								src/ameba/rule/performance/map_instead_of_block.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,52 @@ | |||
| module Ameba::Rule::Performance | ||||
|   # This rule is used to identify usage of `join/sum/product` calls | ||||
|   # that follow `map`. | ||||
|   # | ||||
|   # For example, this is considered inefficient: | ||||
|   # | ||||
|   # ``` | ||||
|   # (1..3).map(&.to_s).join('.') | ||||
|   # (1..3).map(&.*(2)).sum | ||||
|   # ``` | ||||
|   # | ||||
|   # And can be written as this: | ||||
|   # | ||||
|   # ``` | ||||
|   # (1..3).join('.', &.to_s) | ||||
|   # (1..3).sum(&.*(2)) | ||||
|   # ``` | ||||
|   # | ||||
|   # YAML configuration example: | ||||
|   # | ||||
|   # ``` | ||||
|   # Performance/MapInsteadOfBlock: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   class MapInsteadOfBlock < Base | ||||
|     properties do | ||||
|       description "Identifies usage of `join/sum/product` calls that follow `map`." | ||||
|     end | ||||
| 
 | ||||
|     CALL_NAMES = %w(join sum product) | ||||
|     MAP_NAME   = "map" | ||||
|     MSG        = "Use `%s {...}` instead of `map {...}.%s`" | ||||
| 
 | ||||
|     def test(source) | ||||
|       AST::NodeVisitor.new self, source, skip: [ | ||||
|         Crystal::Macro, | ||||
|         Crystal::MacroExpression, | ||||
|         Crystal::MacroIf, | ||||
|         Crystal::MacroFor, | ||||
|       ] | ||||
|     end | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless node.name.in?(CALL_NAMES) && (obj = node.obj) | ||||
|       return unless obj.is_a?(Crystal::Call) && obj.block | ||||
|       return unless obj.name == MAP_NAME | ||||
| 
 | ||||
|       issue_for obj.name_location, node.name_end_location, | ||||
|         MSG % {node.name, node.name} | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -30,16 +30,15 @@ module Ameba::Rule::Performance | |||
|   #     - select | ||||
|   #     - reject | ||||
|   # ``` | ||||
|   # | ||||
|   struct SizeAfterFilter < Base | ||||
|     SIZE_NAME = "size" | ||||
|     MSG       = "Use `count {...}` instead of `%s {...}.#{SIZE_NAME}`." | ||||
| 
 | ||||
|   class SizeAfterFilter < Base | ||||
|     properties do | ||||
|       filter_names : Array(String) = %w(select reject) | ||||
|       description "Identifies usage of `size` calls that follow filter" | ||||
|       filter_names : Array(String) = %w(select reject) | ||||
|     end | ||||
| 
 | ||||
|     SIZE_NAME = "size" | ||||
|     MSG       = "Use `count {...}` instead of `%s {...}.size`." | ||||
| 
 | ||||
|     def test(source) | ||||
|       AST::NodeVisitor.new self, source, skip: [ | ||||
|         Crystal::Macro, | ||||
|  | @ -51,11 +50,10 @@ module Ameba::Rule::Performance | |||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless node.name == SIZE_NAME && (obj = node.obj) | ||||
|       return unless obj.is_a?(Crystal::Call) && obj.block | ||||
|       return unless obj.name.in?(filter_names) | ||||
| 
 | ||||
|       if obj.is_a?(Crystal::Call) && | ||||
|          filter_names.includes?(obj.name) && !obj.block.nil? | ||||
|         issue_for obj.name_location, node.name_end_location, MSG % obj.name | ||||
|       end | ||||
|       issue_for obj.name_location, node.name_end_location, MSG % obj.name | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -21,8 +21,7 @@ module Ameba::Rule::Style | |||
|   # Style/ConstantNames: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct ConstantNames < Base | ||||
|   class ConstantNames < Base | ||||
|     properties do | ||||
|       description "Enforces constant names to be in screaming case" | ||||
|     end | ||||
|  | @ -30,11 +29,11 @@ module Ameba::Rule::Style | |||
|     MSG = "Constant name should be screaming-cased: %s, not %s" | ||||
| 
 | ||||
|     def test(source, node : Crystal::Assign) | ||||
|       if (target = node.target).is_a? Crystal::Path | ||||
|       if (target = node.target).is_a?(Crystal::Path) | ||||
|         name = target.names.first | ||||
|         expected = name.upcase | ||||
| 
 | ||||
|         return if expected == name || name.camelcase == name | ||||
|         return if name.in?(expected, name.camelcase) | ||||
| 
 | ||||
|         issue_for target, MSG % {expected, name} | ||||
|       end | ||||
|  |  | |||
							
								
								
									
										74
									
								
								src/ameba/rule/style/is_a_filter.cr
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										74
									
								
								src/ameba/rule/style/is_a_filter.cr
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,74 @@ | |||
| module Ameba::Rule::Style | ||||
|   # This rule is used to identify usage of `is_a?/nil?` calls within filters. | ||||
|   # | ||||
|   # For example, this is considered invalid: | ||||
|   # | ||||
|   # ``` | ||||
|   # matches = %w[Alice Bob].map(&.match(/^A./)) | ||||
|   # | ||||
|   # matches.any?(&.is_a?(Regex::MatchData)) # => true | ||||
|   # matches.one?(&.nil?)                    # => true | ||||
|   # | ||||
|   # typeof(matches.reject(&.nil?))                    # => Array(Regex::MatchData | Nil) | ||||
|   # typeof(matches.select(&.is_a?(Regex::MatchData))) # => Array(Regex::MatchData | Nil) | ||||
|   # ``` | ||||
|   # | ||||
|   # And it should be written as this: | ||||
|   # | ||||
|   # ``` | ||||
|   # matches = %w[Alice Bob].map(&.match(/^A./)) | ||||
|   # | ||||
|   # matches.any?(Regex::MatchData) # => true | ||||
|   # matches.one?(Nil)              # => true | ||||
|   # | ||||
|   # typeof(matches.reject(Nil))              # => Array(Regex::MatchData) | ||||
|   # typeof(matches.select(Regex::MatchData)) # => Array(Regex::MatchData) | ||||
|   # ``` | ||||
|   # | ||||
|   # YAML configuration example: | ||||
|   # | ||||
|   # ``` | ||||
|   # Style/IsAFilter: | ||||
|   #   Enabled: true | ||||
|   #   FilterNames: | ||||
|   #     - select | ||||
|   #     - reject | ||||
|   #     - any? | ||||
|   #     - all? | ||||
|   #     - none? | ||||
|   #     - one? | ||||
|   # ``` | ||||
|   class IsAFilter < Base | ||||
|     properties do | ||||
|       description "Identifies usage of `is_a?/nil?` calls within filters." | ||||
|       filter_names : Array(String) = %w(select reject any? all? none? one?) | ||||
|     end | ||||
| 
 | ||||
|     MSG = "Use `%s(%s)` instead of `%s {...}`" | ||||
| 
 | ||||
|     def test(source, node : Crystal::Call) | ||||
|       return unless node.name.in?(filter_names) | ||||
|       return unless (block = node.block) | ||||
|       return unless (body = block.body).is_a?(Crystal::IsA) | ||||
|       return unless (path = body.const).is_a?(Crystal::Path) | ||||
|       return unless body.obj.is_a?(Crystal::Var) | ||||
| 
 | ||||
|       name = path.names.join("::") | ||||
|       name = "::#{name}" if path.global? && !body.nil_check? | ||||
| 
 | ||||
|       end_location = node.end_location | ||||
|       if !end_location || end_location.try(&.column_number.zero?) | ||||
|         if end_location = path.end_location | ||||
|           end_location = Crystal::Location.new( | ||||
|             end_location.filename, | ||||
|             end_location.line_number, | ||||
|             end_location.column_number + 1 | ||||
|           ) | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       issue_for node.name_location, end_location, | ||||
|         MSG % {node.name, name, node.name} | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  | @ -4,7 +4,7 @@ module Ameba::Rule::Style | |||
|   # This is considered bad: | ||||
|   # | ||||
|   # ``` | ||||
|   # var.is_a? Nil | ||||
|   # var.is_a?(Nil) | ||||
|   # ``` | ||||
|   # | ||||
|   # And needs to be written as: | ||||
|  | @ -19,8 +19,7 @@ module Ameba::Rule::Style | |||
|   # Style/IsANil: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct IsANil < Base | ||||
|   class IsANil < Base | ||||
|     properties do | ||||
|       description "Disallows calls to `is_a?(Nil)` in favor of `nil?`" | ||||
|     end | ||||
|  | @ -32,7 +31,10 @@ module Ameba::Rule::Style | |||
|       return if node.nil_check? | ||||
| 
 | ||||
|       const = node.const | ||||
|       issue_for const, MSG if const.is_a?(Crystal::Path) && const.names == PATH_NIL_NAMES | ||||
|       return unless const.is_a?(Crystal::Path) | ||||
|       return unless const.names == PATH_NIL_NAMES | ||||
| 
 | ||||
|       issue_for const, MSG | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -26,12 +26,11 @@ module Ameba::Rule::Style | |||
|   #   Enabled: true | ||||
|   #   IntMinDigits: 5 # i.e. integers higher than 9999 | ||||
|   # ``` | ||||
|   # | ||||
|   struct LargeNumbers < Base | ||||
|   class LargeNumbers < Base | ||||
|     properties do | ||||
|       enabled false | ||||
|       description "Disallows usage of large numbers without underscore" | ||||
|       int_min_digits 5 | ||||
|       enabled false | ||||
|     end | ||||
| 
 | ||||
|     MSG = "Large numbers should be written with underscores: %s" | ||||
|  | @ -53,7 +52,7 @@ module Ameba::Rule::Style | |||
|     end | ||||
| 
 | ||||
|     private def allowed?(_sign, value, fraction, _suffix) | ||||
|       return true if !fraction.nil? && fraction.size > 3 | ||||
|       return true if fraction && fraction.size > 3 | ||||
| 
 | ||||
|       digits = value.chars.select &.to_s.=~ /[0-9]/ | ||||
|       digits.size >= int_min_digits | ||||
|  | @ -71,7 +70,7 @@ module Ameba::Rule::Style | |||
|         value.chars.reject(&.== '_').each_slice(by) do |slice| | ||||
|           slices << (yield slice).join | ||||
|         end | ||||
|       end.join("_") | ||||
|       end.join('_') | ||||
|     end | ||||
| 
 | ||||
|     private def parse_number(value) | ||||
|  | @ -83,7 +82,7 @@ module Ameba::Rule::Style | |||
|     end | ||||
| 
 | ||||
|     private def parse_sign(value) | ||||
|       if "+-".includes?(value[0]) | ||||
|       if value[0].in?('+', '-') | ||||
|         sign = value[0] | ||||
|         value = value[1..-1] | ||||
|       end | ||||
|  | @ -91,7 +90,7 @@ module Ameba::Rule::Style | |||
|     end | ||||
| 
 | ||||
|     private def parse_suffix(value) | ||||
|       if pos = (value =~ /e/ || value =~ /_?(i|u|f)/) | ||||
|       if pos = (value =~ /(e|_?(i|u|f))/) | ||||
|         suffix = value[pos..-1] | ||||
|         value = value[0..pos - 1] | ||||
|       end | ||||
|  |  | |||
|  | @ -37,8 +37,7 @@ module Ameba::Rule::Style | |||
|   # Style/MethodNames: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct MethodNames < Base | ||||
|   class MethodNames < Base | ||||
|     properties do | ||||
|       description "Enforces method names to be in underscored case" | ||||
|     end | ||||
|  | @ -51,7 +50,7 @@ module Ameba::Rule::Style | |||
|       line_number = node.location.try &.line_number | ||||
|       column_number = node.name_location.try &.column_number | ||||
| 
 | ||||
|       return if line_number.nil? || column_number.nil? | ||||
|       return unless line_number && column_number | ||||
| 
 | ||||
|       issue_for( | ||||
|         {line_number, column_number}, | ||||
|  |  | |||
|  | @ -26,8 +26,7 @@ module Ameba::Rule::Style | |||
|   # Style/NegatedConditionsInUnless: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct NegatedConditionsInUnless < Base | ||||
|   class NegatedConditionsInUnless < Base | ||||
|     properties do | ||||
|       description "Disallows negated conditions in unless" | ||||
|     end | ||||
|  | @ -35,8 +34,7 @@ module Ameba::Rule::Style | |||
|     MSG = "Avoid negated conditions in unless blocks" | ||||
| 
 | ||||
|     def test(source, node : Crystal::Unless) | ||||
|       return unless negated_condition? node.cond | ||||
|       issue_for node, MSG | ||||
|       issue_for node, MSG if negated_condition?(node.cond) | ||||
|     end | ||||
| 
 | ||||
|     private def negated_condition?(node) | ||||
|  | @ -44,7 +42,7 @@ module Ameba::Rule::Style | |||
|       when Crystal::BinaryOp | ||||
|         negated_condition?(node.left) || negated_condition?(node.right) | ||||
|       when Crystal::Expressions | ||||
|         node.expressions.any? { |e| negated_condition? e } | ||||
|         node.expressions.any? { |e| negated_condition?(e) } | ||||
|       when Crystal::Not | ||||
|         true | ||||
|       else | ||||
|  |  | |||
|  | @ -28,11 +28,10 @@ module Ameba::Rule::Style | |||
|   # Style/PredicateName: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct PredicateName < Base | ||||
|   class PredicateName < Base | ||||
|     properties do | ||||
|       description "Disallows tautological predicate names" | ||||
|       enabled false | ||||
|       description "Disallows tautological predicate names" | ||||
|     end | ||||
| 
 | ||||
|     MSG = "Favour method name '%s?' over '%s'" | ||||
|  |  | |||
|  | @ -55,9 +55,9 @@ module Ameba::Rule::Style | |||
|   # Style/RedundantBegin: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct RedundantBegin < Base | ||||
|   class RedundantBegin < Base | ||||
|     include AST::Util | ||||
| 
 | ||||
|     properties do | ||||
|       description "Disallows redundant begin blocks" | ||||
|     end | ||||
|  | @ -65,9 +65,7 @@ module Ameba::Rule::Style | |||
|     MSG = "Redundant `begin` block detected" | ||||
| 
 | ||||
|     def test(source, node : Crystal::Def) | ||||
|       return unless redundant_begin?(source, node) | ||||
| 
 | ||||
|       issue_for node, MSG | ||||
|       issue_for node, MSG if redundant_begin?(source, node) | ||||
|     end | ||||
| 
 | ||||
|     private def redundant_begin?(source, node) | ||||
|  | @ -76,8 +74,6 @@ module Ameba::Rule::Style | |||
|         redundant_begin_in_handler?(source, body, node) | ||||
|       when Crystal::Expressions | ||||
|         redundant_begin_in_expressions?(body) | ||||
|       else | ||||
|         # nop | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|  | @ -88,7 +84,7 @@ module Ameba::Rule::Style | |||
|     private def redundant_begin_in_handler?(source, handler, node) | ||||
|       return false if begin_exprs_in_handler?(handler) || inner_handler?(handler) | ||||
| 
 | ||||
|       code = node_source(node, source.lines).try &.join("\n") | ||||
|       code = node_source(node, source.lines).try &.join('\n') | ||||
|       def_redundant_begin? code if code | ||||
|     rescue | ||||
|       false | ||||
|  | @ -107,6 +103,7 @@ module Ameba::Rule::Style | |||
|     private def def_redundant_begin?(code) | ||||
|       lexer = Crystal::Lexer.new code | ||||
|       in_body = in_argument_list = false | ||||
| 
 | ||||
|       loop do | ||||
|         token = lexer.next_token | ||||
| 
 | ||||
|  | @ -122,6 +119,7 @@ module Ameba::Rule::Style | |||
|         when :NEWLINE | ||||
|           in_body = true unless in_argument_list | ||||
|         when :SPACE | ||||
|           # ignore | ||||
|         else | ||||
|           return false if in_body | ||||
|         end | ||||
|  |  | |||
|  | @ -96,9 +96,10 @@ module Ameba::Rule::Style | |||
|   #   AllowMultiNext: true | ||||
|   #   AllowEmptyNext: true | ||||
|   # ``` | ||||
|   struct RedundantNext < Base | ||||
|   class RedundantNext < Base | ||||
|     properties do | ||||
|       description "Reports redundant next expressions" | ||||
| 
 | ||||
|       allow_multi_next true | ||||
|       allow_empty_next true | ||||
|     end | ||||
|  |  | |||
|  | @ -93,9 +93,10 @@ module Ameba::Rule::Style | |||
|   #   AllowMutliReturn: true | ||||
|   #   AllowEmptyReturn: true | ||||
|   # ``` | ||||
|   struct RedundantReturn < Base | ||||
|   class RedundantReturn < Base | ||||
|     properties do | ||||
|       description "Reports redundant return expressions" | ||||
| 
 | ||||
|       allow_multi_return true | ||||
|       allow_empty_return true | ||||
|     end | ||||
|  |  | |||
|  | @ -51,8 +51,7 @@ module Ameba::Rule::Style | |||
|   # Style/TypeNames: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct TypeNames < Base | ||||
|   class TypeNames < Base | ||||
|     properties do | ||||
|       description "Enforces type names in camelcase manner" | ||||
|     end | ||||
|  | @ -62,7 +61,7 @@ module Ameba::Rule::Style | |||
|     private def check_node(source, node) | ||||
|       name = node.name.to_s | ||||
|       expected = name.camelcase | ||||
|       return if expected == name | ||||
|       return if name == expected | ||||
| 
 | ||||
|       issue_for node, MSG % {expected, name} | ||||
|     end | ||||
|  |  | |||
|  | @ -42,8 +42,7 @@ module Ameba::Rule::Style | |||
|   # Style/UnlessElse: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct UnlessElse < Base | ||||
|   class UnlessElse < Base | ||||
|     properties do | ||||
|       description "Disallows the use of an `else` block with the `unless`" | ||||
|     end | ||||
|  | @ -51,8 +50,7 @@ module Ameba::Rule::Style | |||
|     MSG = "Favour if over unless with else" | ||||
| 
 | ||||
|     def test(source, node : Crystal::Unless) | ||||
|       return if node.else.nop? | ||||
|       issue_for node, MSG | ||||
|       issue_for node, MSG unless node.else.nop? | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -22,8 +22,7 @@ module Ameba::Rule::Style | |||
|   # Style/VariableNames: | ||||
|   #   Enabled: true | ||||
|   # ``` | ||||
|   # | ||||
|   struct VariableNames < Base | ||||
|   class VariableNames < Base | ||||
|     properties do | ||||
|       description "Enforces variable names to be in underscored case" | ||||
|     end | ||||
|  |  | |||
Some files were not shown because too many files have changed in this diff Show more
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue