Problem in macro create_settings_methods when global Nil class shadow by local with same name

[dammer]

I came across a case where the name of the class Nil is contained in the module in which the class for which we create the settings is defined.

module A
  class Nil
  end

  class B
     Habiat.create do
        setting abc : Array(Something) = Array(Something)
     end
  end
end

In this case, the macro does not work correctly because it refers to a local A::Nil and not a global Nil class.

> 19 | # then raise a MissingSettingError
> 20 | @@abc : Array(Something.class) | Nil  = begin
> 21 |    Array(Something.class).new
          ^
Error: class variable '@@abc' of  A::B::HabitatSettings must be (Array(Something.class) | A::Nil), not (Array(Something.class) | Nil)

Without spoiling the idea of how smart it is to have a separate class Nil inside a local namespace, this can be fixed in code

  setting abc : Array(Something) | Nil | ::Nil  = Array(Something)

but I propose to explicitly specify the global ::Nil in this macro body if this possible.

habitat/src/habitat.cr

Lines 239 to 293 in 4d242eb

	macro create_settings_methods(type_with_habitat)
	{% type_with_habitat = type_with_habitat.resolve %}
	class HabitatSettings
	{% if type_with_habitat.superclass && type_with_habitat.superclass.type_vars.size == 0 && type_with_habitat.superclass.constant(:HABITAT_SETTINGS) %}
	{% for decl in type_with_habitat.superclass.constant(:HABITAT_SETTINGS).map { |setting| setting[:decl] } %}
	def self.{{ decl.var }}
	::{{ type_with_habitat.superclass }}::HabitatSettings.{{ decl.var }}
	end
	{% end %}
	{% end %}
	{% for opt in type_with_habitat.constant(:HABITAT_SETTINGS) %}
	{% decl = opt[:decl] %}
	# NOTE: We can't use the macro level `type.resolve.nilable?` here because
	# there's a few declaration types that don't respond to it which would make the logic
	# more complex. Metaclass, and Proc types are the main, but there may be more.
	{% if decl.type.is_a?(Union) && decl.type.types.map(&.id).includes?(Nil.id) %}
	{% nilable = true %}
	{% else %}
	{% nilable = false %}
	{% end %}
	{% has_default = decl.value || decl.value == false %}
	# Use `begin` to catch if the default value raises an exception,
	# then raise a MissingSettingError
	@@{{ decl.var }} : {{decl.type}} | Nil {% if has_default %} = begin
	{{ decl.value }}
	rescue
	# This will cause a MissingSettingError to be raised
	nil
	end
	{% end %}
	def self.{{ decl.var }}=(value : {{ decl.type }})
	{% if opt[:validation] %}
	{{ type_with_habitat }}.{{ opt[:validation].id }}(value)
	{% end %}
	@@{{ decl.var }} = value
	end
	def self.{{ decl.var }} : {{ decl.type }}
	@@{{ decl.var }}{% if !nilable %}.not_nil!{% end %}
	end
	# Used for checking missing settings on non-nilable types
	# It's advised to use {{ decl.var }} in your apps to ensure
	# the propper type is checked.
	def self.{{ decl.var }}?
	@@{{ decl.var }}
	end
	{% end %}

[jwoertink]

That seems like a reasonable fix; however, I would suggest to not create a class called Nil. Crystal Nil is a struct https://github.com/crystal-lang/crystal/blob/739461fe78c6ce65033f009af3e360750d5b020d/src/nil.cr#L44 and I think having a type with the same name that's treated differently could lead to other issues within the code.

One thing we do in Avram to use an object that means "nothing" but isn't Nil is using a class called Nothing

https://github.com/luckyframework/avram/blob/main/src/avram/nothing.cr

[dammer]

Thanks for the tip with Nothing, great solution, wonder if i can use it as it is a very big legacy shard(