upgrade ameba to v1.3.1 which requires crystal v1.6 (#469)

* upgrade ameba to v1.3.1 which requires crystal v1.6

* bin/ameba --only Style/VerboseBlock --fix

* bin/ameba --only Performance/AnyInsteadOfEmpty --fix

Style/NegatedConditionsInUnless

* bin/ameba --only Performance/ChainedCallWithNoBang

* Use `compact_map {...}` instead of `map {...}.compact`

* reduce string construction time complexity from O(n*n) to O(n)

* increase concurrent amount to catch a potential race condition

* ameba Lint/NotNil: Avoid using `not_nil!`

Dockerfile

@@ -1,4 +1,4 @@
-FROM crystallang/crystal:1.0.0
+FROM crystallang/crystal:1.6.0
 
 ARG sqlite_version=3110000
 ARG sqlite_version_year=2016

shard.yml

@@ -30,4 +30,4 @@ development_dependencies:
 
   ameba:
     github: crystal-ameba/ameba
-    version: ~> 0.13.0
+    version: ~> 1.3.1

spec/adapter/adapters_spec.cr

@@ -15,9 +15,21 @@ describe Granite::Connections do
     it "should allow connections to be be saved and looked up" do
       Granite::Connections.registered_connections.size.should eq 3
 
-      Granite::Connections["mysql"].not_nil!.url.should eq ENV["MYSQL_DATABASE_URL"]
-      Granite::Connections["pg"].not_nil!.url.should eq ENV["PG_DATABASE_URL"]
-      Granite::Connections["sqlite"].not_nil!.url.should eq ENV["SQLITE_DATABASE_URL"]
+      if connection = Granite::Connections["mysql"]
+        connection.url.should eq ENV["MYSQL_DATABASE_URL"]
+      else
+        connection.should_not be_falsey
+      end
+      if connection = Granite::Connections["pg"]
+        connection.url.should eq ENV["PG_DATABASE_URL"]
+      else
+        connection.should_not be_falsey
+      end
+      if connection = Granite::Connections["sqlite"]
+        connection.url.should eq ENV["SQLITE_DATABASE_URL"]
+      else
+        connection.should_not be_falsey
+      end
     end
 
     it "should disallow multiple connections with the same name" do

spec/granite/associations/has_many_spec.cr

@@ -68,9 +68,13 @@ describe "has_many" do
       klass3.teacher = teacher
       klass3.save
 
-      klass = teacher.klasses.find_by(name: "Test class with different name").not_nil!
-      klass.id.should eq klass3.id
-      klass.name.should eq "Test class with different name"
+      klass = teacher.klasses.find_by(name: "Test class with different name")
+      if klass
+        klass.id.should eq klass3.id
+        klass.name.should eq "Test class with different name"
+      else
+        klass.should_not be_nil
+      end
     end
 
     it "#find_by!" do
@@ -93,7 +97,7 @@ describe "has_many" do
       klass3.teacher = teacher
       klass3.save
 
-      klass = teacher.klasses.find_by!(name: "Test class with different name").not_nil!
+      klass = teacher.klasses.find_by!(name: "Test class with different name")
       klass.id.should eq klass3.id
       klass.name.should eq "Test class with different name"
 
@@ -125,9 +129,13 @@ describe "has_many" do
       klass3.teacher = teacher
       klass3.save
 
-      klass = teacher.klasses.find(klass1.id).not_nil!
-      klass.id.should eq klass1.id
-      klass.name.should eq "Test class X"
+      klass = teacher.klasses.find(klass1.id)
+      if klass
+        klass.id.should eq klass1.id
+        klass.name.should eq "Test class X"
+      else
+        klass.should_not be_nil
+      end
     end
 
     it "#find!" do
@@ -150,7 +158,7 @@ describe "has_many" do
       klass3.teacher = teacher
       klass3.save
 
-      klass = teacher.klasses.find!(klass1.id).not_nil!
+      klass = teacher.klasses.find!(klass1.id)
       klass.id.should eq klass1.id
       klass.name.should eq "Test class X"
 

spec/granite/associations/has_many_through_spec.cr

@@ -37,9 +37,9 @@ describe "has_many, through:" do
     enrollment3.student = unrelated_student
     enrollment3.save
 
-    student.klasses.map(&.id).compact.sort.should eq [klass1.id, klass2.id].compact.sort
+    student.klasses.compact_map(&.id).sort!.should eq [klass1.id, klass2.id].compact.sort!
 
-    klass2.students.map(&.id).compact.sort.should eq [student.id, unrelated_student.id].compact.sort
+    klass2.students.compact_map(&.id).sort!.should eq [student.id, unrelated_student.id].compact.sort!
   end
 
   context "querying association" do
@@ -91,9 +91,13 @@ describe "has_many, through:" do
       enrollment3.student = student
       enrollment3.save
 
-      klass = student.klasses.find_by(name: "Test class with different name").not_nil!
-      klass.id.should eq klass3.id
-      klass.name.should eq "Test class with different name"
+      klass = student.klasses.find_by(name: "Test class with different name")
+      if klass
+        klass.id.should eq klass3.id
+        klass.name.should eq "Test class with different name"
+      else
+        klass.should_not be_nil
+      end
     end
 
     it "#find_by!" do
@@ -118,7 +122,7 @@ describe "has_many, through:" do
       enrollment3.student = student
       enrollment3.save
 
-      klass = student.klasses.find_by!(name: "Test class with different name").not_nil!
+      klass = student.klasses.find_by!(name: "Test class with different name")
       klass.id.should eq klass3.id
       klass.name.should eq "Test class with different name"
 
@@ -152,9 +156,13 @@ describe "has_many, through:" do
       enrollment3.student = student
       enrollment3.save
 
-      klass = student.klasses.find(klass1.id).not_nil!
-      klass.id.should eq klass1.id
-      klass.name.should eq "Test class X"
+      klass = student.klasses.find(klass1.id)
+      if klass
+        klass.id.should eq klass1.id
+        klass.name.should eq "Test class X"
+      else
+        klass.should_not be_nil
+      end
     end
 
     it "#find!" do
@@ -179,7 +187,7 @@ describe "has_many, through:" do
       enrollment3.student = student
       enrollment3.save
 
-      klass = student.klasses.find!(klass1.id).not_nil!
+      klass = student.klasses.find!(klass1.id)
       klass.id.should eq klass1.id
       klass.name.should eq "Test class X"
 

spec/granite/querying/all_spec.cr

@@ -9,7 +9,7 @@ describe "#all" do
 
     all = Parent.all
     all.size.should eq model_ids.size
-    all.map(&.id).compact.sort.should eq model_ids.compact
+    all.compact_map(&.id).sort!.should eq model_ids.compact
   end
 
   # TODO Fails under MySQL

spec/granite/querying/find_by_spec.cr

@@ -9,7 +9,11 @@ describe "#find_by, #find_by!" do
     model.save.should be_true
 
     found = Parent.find_by(name: name)
-    found.not_nil!.id.should eq model.id
+    if pa = found
+      pa.id.should eq model.id
+    else
+      pa.should_not be_nil
+    end
 
     found = Parent.find_by!(name: name)
     found.should be_a(Parent)
@@ -23,7 +27,12 @@ describe "#find_by, #find_by!" do
 
     expected = Review.create(name: "review3", upvotes: 10.to_i64)
 
-    Review.find_by(name: "review3", upvotes: 10).not_nil!.id.should eq expected.id
+    r = Review.find_by(name: "review3", upvotes: 10)
+    if r
+      r.id.should eq expected.id
+    else
+      r.should_not be_nil
+    end
 
     expect_raises(Granite::Querying::NotFound, /No .*Review.* found where name = review1 and upvotes = 20/) do
       Review.find_by!(name: "review1", upvotes: 20)
@@ -37,7 +46,11 @@ describe "#find_by, #find_by!" do
     model.save.should be_true
 
     found = Student.find_by(name: nil)
-    found.not_nil!.id.should eq model.id
+    if stu = found
+      stu.id.should eq model.id
+    else
+      stu.should_not be_nil
+    end
 
     found = Student.find_by!(name: nil)
     found.should be_a(Student)
@@ -52,7 +65,11 @@ describe "#find_by, #find_by!" do
     model.save.should be_true
 
     found = ReservedWord.find_by(all: value)
-    found.not_nil!.id.should eq model.id
+    if rw = found
+      rw.id.should eq model.id
+    else
+      rw.should_not be_nil
+    end
 
     found = ReservedWord.find_by!(all: value)
     found.id.should eq model.id
@@ -66,7 +83,11 @@ describe "#find_by, #find_by!" do
     model.save.should be_true
 
     found = Parent.find_by({"name" => name})
-    found.not_nil!.id.should eq model.id
+    if pa = found
+      pa.id.should eq model.id
+    else
+      pa.should_not be_nil
+    end
 
     found = Parent.find_by!({"name" => name})
     found.should be_a(Parent)

spec/granite/querying/find_each_spec.cr

@@ -4,15 +4,15 @@ describe "#find_each" do
   it "finds all the records" do
     Parent.clear
     model_ids = (0...100).map do |i|
-      Parent.new(name: "role_#{i}").tap { |r| r.save }
+      Parent.new(name: "role_#{i}").tap(&.save)
     end.map(&.id)
 
     found_roles = [] of Int64 | Nil
     Parent.find_each do |model|
       found_roles << model.id
     end
 
-    found_roles.compact.sort.should eq model_ids.compact
+    found_roles.compact.sort!.should eq model_ids.compact
   end
 
   it "doesnt yield when no records are found" do
@@ -38,7 +38,7 @@ describe "#find_each" do
       found_models << model.id
     end
 
-    found_models.compact.sort.should eq created_models.compact
+    found_models.compact.sort!.should eq created_models.compact
   end
 
   it "doesnt obliterate a parameterized query" do

spec/granite/querying/find_in_batches.cr

@@ -12,7 +12,7 @@ describe "#find_in_batches" do
       batch.size.should eq 10
     end
 
-    found_models.compact.sort.should eq model_ids.compact
+    found_models.compact.sort!.should eq model_ids.compact
   end
 
   it "doesnt yield when no records are found" do
@@ -56,7 +56,7 @@ describe "#find_in_batches" do
       end
     end
 
-    found_models.compact.sort.should eq created_models.compact
+    found_models.compact.sort!.should eq created_models.compact
   end
 
   it "doesnt obliterate a parameterized query" do
@@ -67,7 +67,7 @@ describe "#find_in_batches" do
     looking_for_ids = created_models[0...5]
 
     Parent.find_in_batches("WHERE id IN(#{looking_for_ids.join(",")})") do |batch|
-      batch.map(&.id).compact.should eq looking_for_ids
+      batch.compact_map(&.id).should eq looking_for_ids
     end
   end
 end

spec/granite/querying/find_spec.cr

@@ -8,7 +8,7 @@ describe "#find, #find!" do
 
     found = Parent.find model.id
     found.should_not be_nil
-    found.not_nil!.id.should eq model.id
+    found && (found.id.should eq model.id)
 
     found = Parent.find! model.id
     found.id.should eq model.id

spec/granite/querying/first_spec.cr

@@ -14,7 +14,11 @@ describe "#first, #first!" do
     end
 
     found = Parent.first
-    found.not_nil!.id.should eq first.id
+    if pa = found
+      pa.id.should eq first.id
+    else
+      pa.should_not be_nil
+    end
 
     found = Parent.first!
     found.id.should eq first.id
@@ -33,7 +37,11 @@ describe "#first, #first!" do
     end
 
     found = Parent.first("ORDER BY id DESC")
-    found.not_nil!.id.should eq second.id
+    if pa = found
+      found.id.should eq second.id
+    else
+      pa.should_not be_nil
+    end
 
     found = Parent.first!("ORDER BY id DESC")
     found.id.should eq second.id

spec/granite/transactions/create_spec.cr

@@ -13,20 +13,19 @@ describe "#create" do
   end
 
   it "doesn't have a race condition on IDs" do
+    n = 1000
+    ids = Array(Int64).new(n)
     channel = Channel(Int64).new
 
-    2.times do
+    n.times do
       spawn do
         parent = Parent.new(name: "Test Parent")
         parent.save
-        channel.send(parent.id.not_nil!)
+        (id = parent.id) && channel.send(id)
       end
     end
-
-    id1 = channel.receive
-    id2 = channel.receive
-
-    id1.should_not eq id2
+    n.times { ids << channel.receive }
+    ids.uniq!.size.should eq n
   end
 
   describe "with a custom primary key" do

spec/granite/transactions/import_spec.cr

@@ -38,7 +38,7 @@ describe "#import" do
 
         reviews = Review.all("WHERE name LIKE ?", ["ImportReview%"])
         reviews.size.should eq 4
-        reviews.none? { |r| r.published }.should be_true
+        reviews.none?(&.published).should be_true
         reviews.all? { |r| r.upvotes == 0 }.should be_true
 
         reviews.each { |r| r.published = true; r.upvotes = 1.to_i64 }
@@ -48,7 +48,7 @@ describe "#import" do
         reviews = Review.all("WHERE name LIKE ?", ["ImportReview%"])
 
         reviews.size.should eq 4
-        reviews.all? { |r| r.published }.should be_true
+        reviews.all?(&.published).should be_true
         reviews.all? { |r| r.upvotes == 1 }.should be_true
       end
     end
@@ -132,7 +132,7 @@ describe "#import" do
         schools.size.should eq 4
         schools.all? { |s| s.name == "ImportExistingSchool" }.should be_true
 
-        schools.each { |s| s.name = "ImportExistingSchoolEdited" }
+        schools.each(&.name=("ImportExistingSchoolEdited"))
 
         School.import(schools, update_on_duplicate: true, columns: ["name"])