Details

    • Type: New Feature New Feature
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.2
    • Fix Version/s: 0.9
    • Component/s: Compiler
    • Labels:
      None
    • Number of attachments :
      0

      Description

      Sometimes Boo is not conservative enough on certain defaults (which still makes sense for other/prototyping usages) that can lead to some errors slipping to a release unnoticed in (large) projects.

      Strict mode introduces more conservative(safer) defaults and warnings.
      Strict mode is disabled by default.

      This compiler option is settable through:

      • "-strict" with booc
      • programatically by setting CompilerParameters.Strict to true
      • "strict" argument with nant and msbuild tasks

      When strict mode is enabled the following changes are enabled:

      • default visibility is private like in C# (change made through BOO-1091)
      • methods parameters types and return type of public methods (API) must be explicitely declared [to avoid unnoticed/unwanted binary-compat breakage] (BOO-1132)
      • warns if a variable has the same name as a private field of one of its super types (BOO-1133).
      • implicit return statement fires a warning (BOO-703)

      TODO:

      • implicit downcast fires a warning (BOO-943) — default mode? or strict mode only?

        Issue Links

          Activity

          Hide
          Cedric Vivier added a comment -

          BOO-703 is also worth considering for strict mode imo.

          Show
          Cedric Vivier added a comment - BOO-703 is also worth considering for strict mode imo.
          Hide
          Rodrigo B. de Oliveira added a comment -

          Field naming convention doesn't seem to belong in there.

          BOO-703 seems to.

          Show
          Rodrigo B. de Oliveira added a comment - Field naming convention doesn't seem to belong in there. BOO-703 seems to.
          Hide
          Cedric Vivier added a comment - - edited

          I was expecting this about the field naming

          On the other hand, it is necessary to have something to guarantee no silent behavior-change wrt conflicts between fields and variables don't you agree?
          As far as I see, either we'd do like python and require "self." for field references, either we'd have to add a keyword for explicit variable declaration.
          Both options would make enabling strict mode source-breaking and less useful in general (since all existing code would require change)

          Since we recommended this naming from the start and that most existing code do follow this rule afaik, don't you think considering "_"-prefix, not much a naming issue but more of a "explicit field reference" is a good compromise?
          Of course this rule would apply only to internal field definitions (and ignore uppercased static/final fields).

          Show
          Cedric Vivier added a comment - - edited I was expecting this about the field naming On the other hand, it is necessary to have something to guarantee no silent behavior-change wrt conflicts between fields and variables don't you agree? As far as I see, either we'd do like python and require "self." for field references, either we'd have to add a keyword for explicit variable declaration. Both options would make enabling strict mode source-breaking and less useful in general (since all existing code would require change) Since we recommended this naming from the start and that most existing code do follow this rule afaik, don't you think considering "_"-prefix, not much a naming issue but more of a "explicit field reference" is a good compromise? Of course this rule would apply only to internal field definitions (and ignore uppercased static/final fields).
          Hide
          Cedric Vivier added a comment -

          Hmm another mostly non-breaking option that would not require preferring one 'recommended naming' among others:

          In strict-mode, disallow to name a variable the same as a field of the declaring type (or one of its super types)

          What do you think of this one?

          Show
          Cedric Vivier added a comment - Hmm another mostly non-breaking option that would not require preferring one 'recommended naming' among others: In strict-mode, disallow to name a variable the same as a field of the declaring type (or one of its super types) What do you think of this one?
          Hide
          Cedric Vivier added a comment -

          Partially landed in rev.3172
          "strict" option added to booc and nant, only changes default visibility for now (other features to follow)

          Show
          Cedric Vivier added a comment - Partially landed in rev.3172 "strict" option added to booc and nant, only changes default visibility for now (other features to follow)
          Hide
          Cedric Vivier added a comment -

          BOO-703 is in.

          Show
          Cedric Vivier added a comment - BOO-703 is in.
          Hide
          Cedric Vivier added a comment -

          BOO-1132 is in.

          Show
          Cedric Vivier added a comment - BOO-1132 is in.
          Hide
          David Piepgrass added a comment -

          I would definitely like a warning for implicit downcasts and implicit return values, but at the same time I strongly prefer inferred return types and default-public access for methods.

          Rationale: implicit downcasts and implicit null return values let you shoot yourself in the foot, whereas return type inference and default-public access are low-risk, oft-used features that are essential to boo's wrist-friendliness. I can see how changing return types affects binary compatibility, but I submit that most developers only need binary compatibility guarantees in a small subset of the code they write.

          Shouldn't this feature be more fine-grained?

          Show
          David Piepgrass added a comment - I would definitely like a warning for implicit downcasts and implicit return values, but at the same time I strongly prefer inferred return types and default-public access for methods. Rationale: implicit downcasts and implicit null return values let you shoot yourself in the foot, whereas return type inference and default-public access are low-risk, oft-used features that are essential to boo's wrist-friendliness. I can see how changing return types affects binary compatibility, but I submit that most developers only need binary compatibility guarantees in a small subset of the code they write. Shouldn't this feature be more fine-grained?
          Hide
          Cedric Vivier added a comment - - edited

          I would definitely like a warning for implicit downcasts and implicit return values, but at the same time I strongly prefer inferred return types and default-public access for methods.

          The warning about inferred return types and parameters types is for API (ie. visible) methods only, by default you do not get this warning since by default all methods are private.
          The rationale about this is that as one should try to pick and publicize only methods that are meant to be used from outside, be easier to document to both team and third-party, and more importantly these methods must not change 'without notice' (like it can with slight changes with inference) since it breaks binary-compatibility.

          only need binary compatibility guarantees in a small subset of the code they write.

          I agree on the large idea (small subset need guarantee) but I disagree with your conclusion, if default method access is public then you probably publish without noticing an unwanted API that is far larger than the "small subset" you intend(ed) to guarantee.
          Also since indeed you generally publicize a small subset only, it makes the 'non-wristfriendliness' to add 'public' to intended public methods and their types less of a burden.

          Shouldn't this feature be more fine-grained?

          It can
          Just use:

          with booc:

           -nowarn:BCW0024 -define:DEFAULT_METHOD_VISIBILITY=public
          

          with nant:

          <booc ....strict="true" define="DEFAULT_METHOD_VISIBILITY=public">
          <nowarn>
               <include>BCW0024</include>
          </nowarn>
          

          or in code:

          import Boo.Lang.Compiler
          macro compilerSettings:
               Parameters.DisableWarning("BCW0024")
               Parameters.DefaultMethodVisibility = TypeMemberModifiers.Public
          

          Alternatively, if strict mode is too strict for your taste, you can just enable BCW0023 (implicit return statement) since it seems that is the only thing that you care about.

          Show
          Cedric Vivier added a comment - - edited I would definitely like a warning for implicit downcasts and implicit return values, but at the same time I strongly prefer inferred return types and default-public access for methods. The warning about inferred return types and parameters types is for API (ie. visible) methods only, by default you do not get this warning since by default all methods are private. The rationale about this is that as one should try to pick and publicize only methods that are meant to be used from outside, be easier to document to both team and third-party, and more importantly these methods must not change 'without notice' (like it can with slight changes with inference) since it breaks binary-compatibility. only need binary compatibility guarantees in a small subset of the code they write. I agree on the large idea (small subset need guarantee) but I disagree with your conclusion, if default method access is public then you probably publish without noticing an unwanted API that is far larger than the "small subset" you intend(ed) to guarantee. Also since indeed you generally publicize a small subset only, it makes the 'non-wristfriendliness' to add 'public' to intended public methods and their types less of a burden. Shouldn't this feature be more fine-grained? It can Just use: with booc: -nowarn:BCW0024 -define:DEFAULT_METHOD_VISIBILITY= public with nant: <booc ....strict= " true " define= "DEFAULT_METHOD_VISIBILITY= public " > <nowarn> <include>BCW0024</include> </nowarn> or in code: import Boo.Lang. Compiler macro compilerSettings: Parameters.DisableWarning( "BCW0024" ) Parameters.DefaultMethodVisibility = TypeMemberModifiers.Public Alternatively, if strict mode is too strict for your taste, you can just enable BCW0023 (implicit return statement) since it seems that is the only thing that you care about.

            People

            • Assignee:
              Cedric Vivier
              Reporter:
              Cedric Vivier
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: