Merge: modelize: ask that attributes in refinements are either noautoninit or have...
authorJean Privat <jean@pryen.org>
Sat, 30 May 2015 00:37:16 +0000 (20:37 -0400)
committerJean Privat <jean@pryen.org>
Sat, 30 May 2015 00:37:16 +0000 (20:37 -0400)
Without value, attributes in introductions does not have the same semantic that attributes in refinements

In introduction attributes are implicitly `autoinit`, in refinements they are `noautoinit`.

This is not POLA since

* this confuse beginners
* readers have to remember if they are in an intro or a refinement
* aditionnal cognitive fragility in constructors (more cases and rules to take in account)

This PR make `autoinit` the default and ask that attributes declared in refinement are either annotated `noautoinit` or have a default value.
This way, the writer has to think about the implication of adding a new attributes in existing classes, especially to think about their initialization. Thus this could help the programmer to avoid bad error.

For the moment, I just display a warning because I want to wait for feedback before doing a big migration (or doing nothing if people hate that), I also want to use jenkins to count and locate these new warnings.

Related to #1322

Pull-Request: #1411
Reviewed-by: Alexis Laferrière <alexis.laf@xymus.net>
Reviewed-by: Alexandre Terrasa <alexandre@moz-code.org>

1  2 
lib/standard/string.nit
src/modelize/modelize_property.nit

diff --combined lib/standard/string.nit
@@@ -898,8 -898,8 +898,8 @@@ abstract class Tex
        #
        # REQUIRE: `n` must be large enough to contain `len` bytes
        #
 -      #       var ns = new NativeString(8)
 -      #       "Text is String".copy_to_native(ns, 8, 2, 0)
 +      #       var ns = new NativeString(8)
 +      #       "Text is String".copy_to_native(ns, 8, 2, 0)
        #       assert ns.to_s_with_length(8) == "xt is St"
        #
        fun copy_to_native(dest: NativeString, n, src_offset, dest_offset: Int) do
@@@ -945,7 -945,7 +945,7 @@@ abstract class FlatTex
        # copy locally the char* as Nit Strings are immutable.
        private fun fast_cstring: NativeString is abstract
  
 -      redef var length: Int = 0
 +      redef var length = 0
  
        redef fun output
        do
@@@ -1194,7 -1194,7 +1194,7 @@@ class FlatStrin
        # Indes in _items of the last item of the string
        private var index_to: Int is noinit
  
 -      redef var chars: SequenceRead[Char] = new FlatStringCharView(self) is lazy
 +      redef var chars = new FlatStringCharView(self) is lazy
  
        redef fun [](index)
        do
                index_to = to
        end
  
 -      redef fun to_cstring: NativeString
 -      do
 +      redef fun to_cstring do
                if real_items != null then
                        return real_items.as(not null)
                else
@@@ -1737,7 -1738,8 +1737,7 @@@ class FlatBuffe
                capacity = c
        end
  
 -      redef fun to_s: String
 -      do
 +      redef fun to_s do
                written = true
                if length == 0 then items = new NativeString(1)
                return new FlatString.with_infos(items, length, 0, length - 1)
@@@ -2035,26 -2037,6 +2035,26 @@@ redef class Boo
        end
  end
  
 +redef class Byte
 +      # C function to calculate the length of the `NativeString` to receive `self`
 +      private fun byte_to_s_len: Int is extern "native_byte_length_str"
 +
 +      # C function to convert an nit Int to a NativeString (char*)
 +      private fun native_byte_to_s(nstr: NativeString, strlen: Int) is extern "native_byte_to_s"
 +
 +      # Displayable byte in its hexadecimal form (0x..)
 +      #
 +      #     assert 1.to_b.to_s       == "0x01"
 +      #     assert (-123).to_b.to_s  == "0x85"
 +      redef fun to_s do
 +              var nslen = byte_to_s_len
 +              var ns = new NativeString(nslen + 1)
 +              ns[nslen] = '\0'
 +              native_byte_to_s(ns, nslen + 1)
 +              return ns.to_s_with_length(nslen)
 +      end
 +end
 +
  redef class Int
  
        # Wrapper of strerror C function
@@@ -2248,12 -2230,6 +2248,12 @@@ redef class Collection[E
        # Concatenate elements.
        redef fun to_s
        do
 +              return plain_to_s
 +      end
 +
 +      # Concatenate element without separators
 +      fun plain_to_s: String
 +      do
                var s = new FlatBuffer
                for e in self do if e != null then s.append(e.to_s)
                return s.to_s
@@@ -2289,7 -2265,7 +2289,7 @@@ en
  redef class Array[E]
  
        # Fast implementation
 -      redef fun to_s
 +      redef fun plain_to_s
        do
                var l = length
                if l == 0 then return ""
@@@ -2476,7 -2452,7 +2476,7 @@@ extern class NativeString `{ char* `
  end
  
  redef class Sys
-       private var args_cache: nullable Sequence[String]
+       private var args_cache: nullable Sequence[String] = null
  
        # The arguments of the program as given by the OS
        fun program_args: Sequence[String]
@@@ -206,9 -206,9 +206,9 @@@ redef class ModelBuilde
                                var mreadpropdef = npropdef.mreadpropdef
                                if mreadpropdef == null or mreadpropdef.msignature == null then return # Skip broken attribute
                                if npropdef.noinit then continue # Skip noinit attribute
 -                              var atautoinit = npropdef.get_single_annotation("autoinit", self)
 -                              if atautoinit != null then
 -                                      # For autoinit attributes, call the reader to force
 +                              var atlateinit = npropdef.get_single_annotation("lateinit", self)
 +                              if atlateinit != null then
 +                                      # For lateinit attributes, call the reader to force
                                        # the lazy initialization of the attribute.
                                        initializers.add(mreadpropdef.mproperty)
                                        mreadpropdef.mproperty.is_autoinit = true
                                        abort
                                end
                        end
 -              else if noautoinit != null then
 -                      if initializers.is_empty then
 -                              warning(noautoinit, "useless-noautoinit", "Warning: the list of autoinit is already empty.")
 -                      end
 -                      # Just clear initializers
 -                      mparameters.clear
 -                      initializers.clear
                else
                        # Search the longest-one and checks for conflict
                        var longest = spropdefs.first
                        if spropdefs.length > 1 then
 -                              # Check for conflict in the order of initializers
 -                              # Each initializer list must me a prefix of the longest list
                                # part 1. find the longest list
                                for spd in spropdefs do
                                        if spd.initializers.length > longest.initializers.length then longest = spd
                                end
                                # part 2. compare
 -                              for spd in spropdefs do
 +                              # Check for conflict in the order of initializers
 +                              # Each initializer list must me a prefix of the longest list
 +                              # If `noautoinit` is set, just ignore conflicts
 +                              if noautoinit == null then for spd in spropdefs do
                                        var i = 0
                                        for p in spd.initializers do
                                                if p != longest.initializers[i] then
                                end
                        end
  
 -                      # Can we just inherit?
 -                      if spropdefs.length == 1 and mparameters.is_empty and defined_init == null then
 -                              self.toolcontext.info("{mclassdef} inherits the basic constructor {longest}", 3)
 -                              mclassdef.mclass.root_init = longest
 -                              return
 -                      end
 +                      if noautoinit != null then
 +                              # If there is local or inherited initializers, then complain.
 +                              if initializers.is_empty and longest.initializers.is_empty then
 +                                      warning(noautoinit, "useless-noautoinit", "Warning: the list of autoinit is already empty.")
 +                              end
 +                              # Just clear initializers
 +                              mparameters.clear
 +                              initializers.clear
 +                      else
 +                              # Can we just inherit?
 +                              if spropdefs.length == 1 and mparameters.is_empty and defined_init == null then
 +                                      self.toolcontext.info("{mclassdef} inherits the basic constructor {longest}", 3)
 +                                      mclassdef.mclass.root_init = longest
 +                                      return
 +                              end
  
 -                      # Combine the inherited list to what is collected
 -                      if longest.initializers.length > 0 then
 -                              mparameters.prepend longest.new_msignature.mparameters
 -                              initializers.prepend longest.initializers
 +                              # Combine the inherited list to what is collected
 +                              if longest.initializers.length > 0 then
 +                                      mparameters.prepend longest.new_msignature.mparameters
 +                                      initializers.prepend longest.initializers
 +                              end
                        end
                end
  
@@@ -660,8 -656,6 +660,8 @@@ redef class APropde
                return true
        end
  
 +      # Checks for useless type in redef signatures.
 +      private fun check_repeated_types(modelbuilder: ModelBuilder) do end
  end
  
  redef class ASignature
@@@ -1057,28 -1051,6 +1057,28 @@@ redef class AMethPropde
                        var nt = nsig.n_type
                        if nt != null then modelbuilder.check_visibility(nt, nt.mtype.as(not null), mpropdef)
                end
 +              check_repeated_types(modelbuilder)
 +      end
 +
 +      # For parameters, type is always useless in a redef.
 +      # For return type, type is useless if not covariant with introduction.
 +      redef fun check_repeated_types(modelbuilder) do
 +              if mpropdef.is_intro or n_signature == null then return
 +              # check params
 +              for param in n_signature.n_params do
 +                      if param.n_type != null then
 +                              modelbuilder.advice(param.n_type, "useless-signature", "Warning: useless type repetition on parameter `{param.n_id.text}` for redefined method `{mpropdef.name}`")
 +                      end
 +              end
 +              # get intro
 +              var intro = mpropdef.mproperty.intro
 +              var n_intro = modelbuilder.mpropdef2npropdef.get_or_null(intro)
 +              if n_intro == null or not n_intro isa AMethPropdef then return
 +              # check return type
 +              var ret_type = n_signature.ret_type
 +              if ret_type != null and ret_type == n_intro.n_signature.ret_type then
 +                      modelbuilder.advice(n_signature.n_type, "useless-signature", "Warning: useless return type repetition for redefined method `{mpropdef.name}`")
 +              end
        end
  end
  
@@@ -1215,17 -1187,17 +1215,17 @@@ redef class AAttrPropde
                end
  
                var atlazy = self.get_single_annotation("lazy", modelbuilder)
 -              var atautoinit = self.get_single_annotation("autoinit", modelbuilder)
 -              if atlazy != null or atautoinit != null then
 -                      if atlazy != null and atautoinit != null then
 -                              modelbuilder.error(atlazy, "Error: `lazy` incompatible with `autoinit`.")
 +              var atlateinit = self.get_single_annotation("lateinit", modelbuilder)
 +              if atlazy != null or atlateinit != null then
 +                      if atlazy != null and atlateinit != null then
 +                              modelbuilder.error(atlazy, "Error: `lazy` incompatible with `lateinit`.")
                                return
                        end
                        if not has_value then
                                if atlazy != null then
                                        modelbuilder.error(atlazy, "Error: `lazy` attributes need a value.")
 -                              else if atautoinit != null then
 -                                      modelbuilder.error(atautoinit, "Error: `autoinit` attributes need a value.")
 +                              else if atlateinit != null then
 +                                      modelbuilder.error(atlateinit, "Error: `lateinit` attributes need a value.")
                                end
                                has_value = true
                                return
                        return
                end
  
+               if not mclassdef.is_intro and not has_value and not noinit then
+                       modelbuilder.advice(self, "attr-in-refinement", "Warning: attributes in refinement need a value or `noautoinit`.")
+               end
                var writename = name + "="
                var atwritable = self.get_single_annotation("writable", modelbuilder)
                if atwritable != null then
                                else if nexpr isa AIntExpr then
                                        var cla = modelbuilder.try_get_mclass_by_name(nexpr, mmodule, "Int")
                                        if cla != null then mtype = cla.mclass_type
 +                              else if nexpr isa AByteExpr then
 +                                      var cla = modelbuilder.try_get_mclass_by_name(nexpr, mmodule, "Byte")
 +                                      if cla != null then mtype = cla.mclass_type
                                else if nexpr isa AFloatExpr then
                                        var cla = modelbuilder.try_get_mclass_by_name(nexpr, mmodule, "Float")
                                        if cla != null then mtype = cla.mclass_type
                if mlazypropdef != null then
                        mlazypropdef.static_mtype = modelbuilder.model.get_mclasses_by_name("Bool").first.mclass_type
                end
 +              check_repeated_types(modelbuilder)
        end
  
        redef fun check_signature(modelbuilder)
                        end
                end
        end
 +
 +      # Type is useless if the attribute type is the same thant the intro.
 +      redef fun check_repeated_types(modelbuilder) do
 +              if mreadpropdef.is_intro or n_type == null then return
 +              # get intro
 +              var intro = mreadpropdef.mproperty.intro
 +              var n_intro = modelbuilder.mpropdef2npropdef.get_or_null(intro)
 +              if n_intro == null then return
 +              # get intro type
 +              var ntype = null
 +              if n_intro isa AMethPropdef then
 +                      ntype = n_intro.n_signature.ret_type
 +              else if n_intro isa AAttrPropdef and n_intro.n_type != null then
 +                      ntype = n_intro.n_type.mtype
 +              end
 +              # check
 +              if ntype ==null or ntype != n_type.mtype then return
 +              modelbuilder.advice(n_type, "useless-signature", "Warning: useless type repetition on redefined attribute `{mpropdef.name}`")
 +      end
  end
  
  redef class ATypePropdef