Uploaded image for project: 'Puppet'
  1. Puppet
  2. PUP-4128

Add migration check for significant whitespace

    Details

    • Type: New Feature
    • Status: Closed
    • Priority: Normal
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: PUP 3.8.0, PUP 4.1.0
    • Component/s: None
    • Labels:
      None
    • Template:
    • Story Points:
      1
    • Sprint:
      2013-08-12 Sprint

      Description

      In 4.x white space is significant in one place; to get an index/key from an Array/Hash, or make a reference to an instance of a resource, requires that the opening [ is not separated by whitespace from the LHS operand. If it is, the result is two separate expressions, where the second is a LiteralArray.

      It is expected that user will get 4.x validation errors, but there are corner cases where logic can return a very different result. Again, this is likely to lead to errors, or being catched by one of the other migration validations. This is one corner case:

      if true {
        $a = File ['foo']
      }
      

      This leads to $a being set to Foo in 4x, and File['foo'] in 3x.

      Most cases will result in an error 'expression has no effect', but if it happens to be the last in a sequence (as in the if-then block above), then it is allowed as the (second expression) literal array forms the produced value of the if-expression.

      Since no 3x logic could possibly expect a block/sequence to produce a literal value, we can add migration validation that "no block may end with an 'non-productive' expression". The cause of this (for a valid 3x program) is probably always the significant whitespace issue.

      Implementation

      The easiest implementation is to modify the checker4_0.rb method check_BlockExpression(o) and add an assertion that the last expression is not a LiteralList if there is a MigrationChecker in effect. This since all other cases where a significant whitespace alters the meaning of the program results in the error IDEM_EXPRESSION_NOT_LAST or IDEM_NOT_ALLOWED_LAST (if it is last in a class or define).

      MIGRATE4_ARRAY_LAST_IN_BLOCK since it is specific to migration. The message could have the text "The expression parsed to an Array (3.x parses this as an [] operation on the preceding value even if there is white space before the Array)"

      Implement the assertion and reporting in a method report_array_last_in_block(last_expr) in MigrationChecker to not clutter the main implementation and make it easier to remove this when merging to master.

      (Add method stubs to the MigrationChecker in the puppet repository and the concrete methods to the implementation in the module created in PUP-4149).

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                henrik.lindberg Henrik Lindberg
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Zendesk Support