Avoid using getter! and property! unless they're expected to not …

…be nil

spec/spectator/core/item_spec.cr

@@ -16,7 +16,7 @@ Spectator.describe Spectator::Core::Item do
 
     it "is nil if the item does not have a description" do
       item = TestItem.new
-      expect(item.description?).to be_nil
+      expect(item.description).to be_nil
     end
 
     it "is the result of #inspect when passing a non-string to #initialize" do

spec/spectator/core/location_spec.cr

@@ -5,25 +5,25 @@ Spectator.describe Spectator::Core::Location do
     it "parses a location" do
       location = Spectator::Core::Location.parse("foo:10")
       expect(location.file).to eq("foo")
-      expect(location.line?).to eq(10)
+      expect(location.line).to eq(10)
     end
 
     it "parses a location with no line number" do
       location = Spectator::Core::Location.parse("foo")
       expect(location.file).to eq("foo")
-      expect(location.line?).to be_nil
+      expect(location.line).to be_nil
     end
 
     it "parses a Windows path with a colon" do
       location = Spectator::Core::Location.parse("C:\\foo:10")
       expect(location.file).to eq("C:\\foo")
-      expect(location.line?).to eq(10)
+      expect(location.line).to eq(10)
     end
 
     it "parses a Windows path with no line number" do
       location = Spectator::Core::Location.parse("C:\\foo")
       expect(location.file).to eq("C:\\foo")
-      expect(location.line?).to be_nil
+      expect(location.line).to be_nil
     end
   end
 

src/spectator/core/configuration.cr

@@ -55,7 +55,7 @@ module Spectator
 
       property order = Order::Defined
 
-      property! seed : UInt64
+      property seed : UInt64?
 
       property error_exit_code = 1
     end

src/spectator/core/context_hook.cr

@@ -7,9 +7,9 @@ module Spectator::Core
       After
     end
 
-    getter! location : LocationRange
+    getter location : LocationRange?
 
-    getter! exception : Exception
+    getter exception : Exception?
 
     getter position : Position
 

src/spectator/core/example.cr

@@ -17,7 +17,7 @@ module Spectator::Core
     def run : Result
       @run = true
       @previous_result = Result.capture do
-        if context = parent?
+        if context = parent
           context.with_hooks(self) do
             @block.call(self)
           end
@@ -30,7 +30,7 @@ module Spectator::Core
     # Indicates if the example has been run.
     getter? run : Bool = false
 
-    getter! previous_result : Result
+    getter previous_result : Result?
 
     def group? : ExampleGroup?
       parent?.try &.as?(ExampleGroup)

src/spectator/core/example_group.cr

@@ -20,7 +20,7 @@ module Spectator::Core
 
     def add_child(child : Item) : Nil
       # Attempt to remove the child from its previous parent.
-      child.parent?.try do |prev_parent|
+      child.parent.try do |prev_parent|
         prev_parent.remove_child(child) if prev_parent.responds_to?(:remove_child)
       end
 
@@ -32,7 +32,7 @@ module Spectator::Core
       @children.delete(child)
 
       # Disassociate the child only if it's ours.
-      child.parent = nil if child.parent? == self
+      child.parent = nil if child.parent == self
     end
 
     def each(& : Item ->) : Nil

src/spectator/core/example_hook.cr

@@ -8,9 +8,9 @@ module Spectator::Core
       Around
     end
 
-    getter! location : LocationRange
+    getter location : LocationRange?
 
-    getter! exception : Exception
+    getter exception : Exception?
 
     getter position : Position
 

src/spectator/core/hooks.cr

@@ -129,7 +129,7 @@ module Spectator::Core
     # TODO: around_all
 
     protected def with_hooks(example : Example, &block : ->) : Nil
-      if context = parent?
+      if context = parent
         context.with_hooks(example) do
           with_current_context_hooks(example, &block)
         end

src/spectator/core/item.cr

@@ -6,34 +6,34 @@ module Spectator::Core
   abstract class Item
     # The description of the item.
     # This may be nil if the item does not have a description.
-    getter! description : String
+    getter description : String?
 
     # The full description of the item.
     # This is a combination of all parent descriptions and this item's description.
     def full_description : String?
-      return unless description?
+      return unless description
       String.build do |io|
         full_description(io)
       end
     end
 
     # Appends the parent's description and this item's description to the given *io*.
     def full_description(io : IO) : Nil
-      return unless description?
-      if parent = parent?
-        parent.full_description(io)
-        io << ' ' if parent.description?
+      return unless description
+      parent.try do |context|
+        context.full_description(io)
+        io << ' ' if context.description
       end
       io << description
     end
 
     # The location of the item in the source code.
     # This may be nil if the item does not have a location,
     # such as if it was dynamically generated.
-    getter! location : LocationRange
+    getter location : LocationRange?
 
     # Context (the example group) the item belongs to.
-    getter! parent : Context
+    getter parent : Context?
 
     # Sets the context (the example group) the item belongs to.
     # NOTE: It is important that the context should be made aware that this item is a child.

src/spectator/core/location.cr

@@ -5,7 +5,7 @@ module Spectator::Core
     getter file : String
 
     # Line number in the source file.
-    getter! line : Int32
+    getter line : Int32?
 
     # Creates a new location.
     def initialize(@file, @line = nil)

src/spectator/core/location_range.cr

@@ -31,7 +31,7 @@ module Spectator::Core
     # Checks if the location is in the range.
     def includes?(location : Location)
       return false if location.file != @file
-      return true unless line = location.line?
+      return true unless line = location.line
       @line <= line && line <= @end_line
     end
 

src/spectator/core/runner.cr

@@ -58,7 +58,7 @@ module Spectator
     end
 
     class Sandbox
-      property! current_example : Example
+      property current_example : Example?
       getter root_example_group = ExampleGroup.new
       getter path : Path = Path[Dir.current]
 

src/spectator/formatters/compatible_junit_formatter.cr

@@ -80,8 +80,8 @@ module Spectator::Formatters
 
       @test_cases << if result.failed?
         JUnit::FailedTestCase.new(**properties, error: result.exception)
-      elsif result.skipped?
-        JUnit::SkippedTestCase.new(**properties, skip_message: result.error_message)
+      elsif result.skipped? && (skip_message = result.error_message)
+        JUnit::SkippedTestCase.new(**properties, skip_message: skip_message)
       else
         JUnit::PassedTestCase.new(**properties)
       end

src/spectator/matchers/built_in/raise_error_matcher.cr

@@ -3,7 +3,7 @@ module Spectator::Matchers::BuiltIn
     @expected_error : T?
     @expected_message : String | Regex?
 
-    getter! rescued_error : Exception
+    getter rescued_error : Exception?
 
     def initialize(@expected_error : T)
     end

src/spectator/matchers/expect.cr

@@ -64,7 +64,7 @@ module Spectator::Matchers
         failure_message: failure_message,
         location: location)
       match_data.try_raise
-      matcher.rescued_error
+      matcher.rescued_error.not_nil!("BUG: Error should have been captured")
     end
 
     def not_to(matcher, failure_message : String? = nil, *,