Merge pull request #3 from wyhaines/kh.workaround_macro_expression_20…

…220422

Kh.workaround macro expression 20220422

.github/workflows/ci.yml

@@ -21,8 +21,8 @@ jobs:
       run: shards install
     - name: Run tests
       run: crystal spec -t -s
-#    - name: Run Ameba
-#      run: bin/ameba
+    - name: Run Ameba
+      run: bin/ameba
     - name: Build docs
       run: crystal docs
     - name: Deploy

README.md

@@ -121,8 +121,19 @@ class VerMe
     puts "WARNING -- Using version of Inner less than 0.2.0 is deprecated, and support will be removed"
   end
 end
+
+class MacrosAreTough
+  VERSION = {{ `shards version "#{__DIR__}"`.chomp.stringify }}
+end
+
+if_version?(MacrosAreTough, :>=, "0.1.0") do
+  puts "This will only run if the version of MacrosAreTough is >= 0.1.0"
+end
 ```
 
+The last example, with `MacrosAreTough`, [deserves additional explanation](https://wyhaines.github.io/defined.cr/toplevel.html#if_version%3F%28const%2Ccomparison%2Cvalue%2C%26code%29-macro), because inline macro expressions
+like that pose a particular challenge for the library, and there are some edge cases.
+
 ### Environment Variable Checks
 
 This facility provides four very simple macros (`if_enabled?`, `unless_enabled?`, `if_disabled?`, `unless_disabled?`) which can be used to conditionally compile code based on whether an environment variable is set, or not. The macros use a shell based definition of truthy and falsey, so `0` and `false` and `""` are considered false, while all other values are true.
@@ -154,6 +165,10 @@ unless_disabled?("ENV_VAR") do
 end
 ```
 
+## More Documentation
+
+[https://wyhaines.github.io/defined.cr/toplevel.html](https://wyhaines.github.io/defined.cr/toplevel.html)
+
 ## Contributing
 
 1. Fork it (<https://github.com/wyhaines/defined.cr/fork>)

shard.yml

@@ -1,5 +1,5 @@
 name: defined
-version: 0.3.2
+version: 0.3.3
 
 authors:
   - Kirk Haines <[email protected]>
@@ -11,4 +11,4 @@ license: MIT
 development_dependencies:
   ameba:
     github: crystal-ameba/ameba
-    version: ~> 0.14
+    version: ~> 1.0.0

spec/defined_spec.cr

@@ -67,4 +67,12 @@ describe Defined do
       true.should be_true
     end
   end
+
+  it "if_version? can handle a constant that resolves back to a MacroVersion" do
+    DefinedTestResults::Answers[:workaround_if].should be_true
+  end
+
+  it "unless_version? can handle a constant that resolves back to a MacroVersion" do
+    DefinedTestResults::Answers[:workaround_unless].should be_true
+  end
 end

spec/spec_helper.cr

@@ -26,6 +26,10 @@ end
 class Bif
 end
 
+class Workaround
+  VERSION = {{ `shards version "#{__DIR__}"`.chomp.stringify }}
+end
+
 class DefinedTestResults
   Answers = {} of Symbol => Bool
 
@@ -53,6 +57,16 @@ class DefinedTestResults
   unless_version?(VerMe, :>, "2.0.0") do
     Answers[:unless] = true
   end
+
+  Answers[:workaround_if] = false
+  if_version?(Workaround, :>, "0.0.1") do
+    Answers[:workaround_if] = true
+  end
+
+  Answers[:workaround_unless] = false
+  unless_version?(Workaround, :>, "99.99.99") do
+    Answers[:workaround_unless] = true
+  end
 end
 
 DefinedTestResults::Answers[:partially_qualified_relative_path] = false

src/defined.cr

@@ -1,9 +1,16 @@
 require "./version"
 
-# This macro accepts a string or a symbol of a fully qualified constant name.
+# This macro accepts a consonant, or a string or symbol of the consonant name.
 # It validates whether the constant is defined, starting at the top level. The value
 # of the constant will be returned if it is defined. If it is not defined, `false` is
 # returned.
+#
+# ```
+# has_db = defined?("DB")
+#
+# puts "Yes, DB was required." if has_db
+# ```
+#
 macro defined?(const)
   {%
     parts = [] of String
@@ -37,10 +44,17 @@ macro defined?(const)
   {% end %}
 end
 
-# This macro accepts a string or a symbol of a fully qualified constant name.
+# This macro accepts a constant, or a string or a symbol of a fully qualified constant name.
 # It validates whether the constant is defined, starting at the top level. If the
 # constant is defined, the code passed to the macro via a block will be instantiated.
 # This permits conditional code evaluation based on whether a constant is defined.
+#
+# ```
+# if_defined?("MyClass::FeatureFlag") do
+#   Log.info { "MyClass::FeatureFlag has been enabled" }
+# end
+# ```
+#
 macro if_defined?(const, &code)
   {%
     parts = [] of String
@@ -72,10 +86,17 @@ macro if_defined?(const, &code)
   {% end %}
 end
 
-# This macro accepts a string or a symbol of a fully qualified constant name.
+# This macro accepts a constant, or a string or a symbol of a fully qualified constant name.
 # It validates whether the constant is defined, starting at the top level. If the
 # constant is not defined, the code passed to the macro via a block will be instantiated.
 # This permits conditional code evaluation based on whether a constant is defined.
+#
+# ```
+# unless_defined?("SpecialLibrary") do
+#   Workaround.configure
+# end
+# ```
+#
 macro unless_defined?(const, &code)
   {%
     parts = [] of String
@@ -107,11 +128,63 @@ macro unless_defined?(const, &code)
   {% end %}
 end
 
-# This macro accepts a string or a symbol of a fully qualified constant name.
+# This macro accepts a constant, or a string or a symbol of a fully qualified constant name.
 # This constant will be checked for a `VERSION` or a `Version` constant, or a
 # `#version` method under it. If it exists, the value held by that constant, or
 # returned by the `#version` method is compared with the provided comparison operator
 # to the value, using a SemanticVersion comparison.
+#
+# ```
+# if_version?("Crystal", :>, "1.0.0") do
+#   # Do a thing that only works on Crystal 1.0.0 and later
+# end
+# ```
+#
+# #### Caveats
+#
+# If the version is defined using a macro expression (macro code enclosed in a `{{ ... }}` block),
+# things become more difficult. The `compare_versions` macro expects to to receive a StringLiteral,
+# SymbolLiteral, or MacroID. If it receives a MacroExpression, it can not evaluate that expression
+# to access the value that it returns, and an exception is thrown when the macro is evaluated.
+# To make things more interesting, there is no way to force that MacroExpression to be expanded
+# from within macro code, making it difficult to access the value. So, for instance, if there were
+# the following version definition:
+#
+# ```
+# class Foo
+#   VERSION = {{ `shards version "#{__DIR__}"`.chomp.stringify }}
+# end
+# ```
+#
+# Then the `if_version?` macro would not be able to access the value of `VERSION` because it
+# would be a MacroExpression.
+#
+# This library does have a workaround for that situation, which will work for simple cases like the
+# above example, however. Essentially, when it encounters a MacroExpression, it reformulates the
+# constant into the local scope, with evaluation wrapped by a `{% begin %} ... {% end %}` block.
+# Wrapping the macro expression in that way ensures that the value of the expression is already
+# assigned to the constant when the `if_version?` macro is evaluated.
+# So, `if_version?` used on Foo, might look like this:
+#
+# ```
+# if_version?("Foo", :>, "1.0.0") do
+#   # Awesome. That's a great version of Foo!
+# end
+# ```
+#
+# The macro will rewrite that to look something like this:
+#
+# ```
+# {% begin %}
+# X__temp_731 = {{ `shards version "#{__DIR__}"`.chomp.stringify }}
+# {% end %}
+# if_version?("Foo", :>, "1.0.0") do
+#   # Awesome. That's a great version of Foo!
+# end
+# ```
+#
+# And this *will* work.
+#
 macro if_version?(const, comparison, value, &code)
   {%
     parts = [] of String
@@ -128,8 +201,8 @@ macro if_version?(const, comparison, value, &code)
         const.id.gsub(/^::/, "").split("::").all? do |part|
           clean_part = part.tr(":", "").id
           parts << clean_part
-          if position && position.has_constant?(clean_part.id)
-            found_position = position.constant(clean_part.id)
+          if position && position.resolve.has_constant?(clean_part.id)
+            found_position = position.resolve.constant(clean_part.id)
             do_break = true
           else
             position = false
@@ -139,18 +212,24 @@ macro if_version?(const, comparison, value, &code)
     end
 
     result = false
+    do_nested_version = false
+    full_const = nil
     if found_position
       if found_position.is_a?(StringLiteral)
         version = found_position
-      elsif found_position.has_constant?(:VERSION)
-        version = found_position.constant(:VERSION)
-      elsif found_position.has_constant?(:Version)
-        version = found_position.constant(:Version)
+      elsif found_position.resolve.has_constant?(:VERSION)
+        full_const = "#{const.id}::VERSION"
+        version = found_position.resolve.constant(:VERSION)
+      elsif found_position.resolve.has_constant?(:Version)
+        full_const = "#{const.id}::Version"
+        version = found_position.resolve.constant(:Version)
       else
         version = false
       end
 
-      if version
+      if version.is_a?(MacroExpression)
+        do_nested_version = true
+      elsif version
         cmpx = compare_versions(version, value)
         if comparison.id == ">"
           result = cmpx == 1
@@ -166,11 +245,20 @@ macro if_version?(const, comparison, value, &code)
       end
     end
   %}
-  {% if result %}
+  {% if do_nested_version %}
+    \{% begin %}
+    {{ "X".id }}%cnst = {{ version.id }}
+    \{% end %}
+    if_version?( {{ "X".id }}%cnst, {{ comparison }}, {{ value.stringify.id }}) do
+      {{ code.body }}
+    end
+  {% elsif result %}
     {{ code.body }}
   {% end %}
 end
 
+# See the documentation for `#if_version?`.
+#
 macro unless_version?(const, comparison, value, &code)
   {%
     parts = [] of String
@@ -187,8 +275,8 @@ macro unless_version?(const, comparison, value, &code)
         const.id.gsub(/^::/, "").split("::").all? do |part|
           clean_part = part.tr(":", "").id
           parts << clean_part
-          if position && position.has_constant?(clean_part.id)
-            found_position = position.constant(clean_part.id)
+          if position && position.resolve.has_constant?(clean_part.id)
+            found_position = position.resolve.constant(clean_part.id)
             do_break = true
           else
             position = false
@@ -198,18 +286,24 @@ macro unless_version?(const, comparison, value, &code)
     end
 
     result = false
+    do_nested_version = false
+    full_const = nil
     if found_position
       if found_position.is_a?(StringLiteral)
         version = found_position
-      elsif found_position.has_constant?(:VERSION)
-        version = found_position.constant(:VERSION)
-      elsif found_position.has_constant?(:Version)
-        version = found_position.constant(:Version)
+      elsif found_position.resolve.has_constant?(:VERSION)
+        full_const = "#{const.id}::VERSION"
+        version = found_position.resolve.constant(:VERSION)
+      elsif found_position.resolve.has_constant?(:Version)
+        full_const = "#{const.id}::Version"
+        version = found_position.resolve.constant(:Version)
       else
         version = false
       end
 
-      if version
+      if version.is_a?(MacroExpression)
+        do_nested_version = true
+      elsif version
         cmpx = compare_versions(version, value)
         if comparison.id == ">"
           result = cmpx == 1
@@ -225,7 +319,14 @@ macro unless_version?(const, comparison, value, &code)
       end
     end
   %}
-  {% unless result %}
+  {% if do_nested_version %}
+    \{% begin %}
+    {{ "X".id }}%cnst = {{ version.id }}
+    \{% end %}
+    unless_version?( {{ "X".id }}%cnst, {{ comparison }}, {{ value.stringify.id }}) do
+      {{ code.body }}
+    end
+  {% elsif !result %}
     {{ code.body }}
   {% end %}
 end

src/version.cr

@@ -1,3 +1,3 @@
 module Defined
-  VERSION = "0.3.2"
+  VERSION = "0.3.3"
 end