[Backport] (PUP-12026) Change non-literal type check to deprecation warning

lib/puppet/info_service/class_information_service.rb

@@ -74,7 +74,7 @@ def extract_param(p)
 
  def extract_type(structure, p)
  return structure if p.type_expr.nil?
- structure[:type] = typeexpr_to_string(p.type_expr)
+ structure[:type] = typeexpr_to_string(p.name, p.type_expr)
  structure
  end
 
@@ -87,12 +87,11 @@ def extract_default(structure, p)
  structure
  end
 
- def typeexpr_to_string(type_expr)
+ def typeexpr_to_string(name, type_expr)
  begin
  type_parser.interpret_any(type_expr, nil).to_s
- rescue Puppet::ParseError
- # type is to complex - contains expressions that are not literal
- nil
+ rescue Puppet::ParseError => e
+ raise Puppet::Error, "The parameter '$#{name}' is invalid: #{e.message}", e.backtrace
  end
  end
 

lib/puppet/pops/validation/validator_factory_4_0.rb

@@ -37,7 +37,7 @@ def severity_producer
  p[Issues::EMPTY_RESOURCE_SPECIALIZATION] = :ignore
  p[Issues::CLASS_NOT_VIRTUALIZABLE] = :error
 
- p[Issues::ILLEGAL_NONLITERAL_PARAMETER_TYPE] = Puppet[:strict] == :off ? :ignore : Puppet[:strict]
+ p[Issues::ILLEGAL_NONLITERAL_PARAMETER_TYPE] = :deprecation
  p
  end
 end

spec/lib/puppet_spec/matchers.rb

@@ -14,6 +14,12 @@
  end
 end
 
+RSpec::Matchers.define :have_matching_log_with_source do |expected, file, line, pos|
+ match do |actual|
+ actual.any? { |item| item.message =~ expected && item.file == file && item.line == line && item.pos == pos }
+ end
+end
+
 RSpec::Matchers.define :exit_with do |expected|
  actual = nil
  match do |block|

spec/unit/info_service_spec.rb

@@ -518,20 +518,23 @@ class oops_2(Optional[[String]] $double_brackets) { }
  it "warns with a descriptive message if non-literal class parameter is given" do
  files = ['non_literal.pp', 'non_literal_2.pp'].map {|f| File.join(code_dir, f) }
  Puppet::InfoService.classes_per_environment({'production' => files })
- expect(@logs).to include(an_object_having_attributes(message: "The parameter '$bad_int' must be a literal type, not a Puppet::Pops::Model::AccessExpression"))
- expect(@logs).to include(an_object_having_attributes(message: "The parameter '$double_brackets' must be a literal type, not a Puppet::Pops::Model::AccessExpression"))
+ expect(@logs).to have_matching_log_with_source(/The parameter '\$bad_int' must be a literal type, not a Puppet::Pops::Model::AccessExpression/, "#{code_dir}/non_literal.pp", 1, 37)
+ expect(@logs).to have_matching_log_with_source(/The parameter '\$double_brackets' must be a literal type, not a Puppet::Pops::Model::AccessExpression/, "#{code_dir}/non_literal_2.pp", 1, 44)
  end
 
  it "errors in strict mode if non-literal class parameter is given" do
  Puppet[:strict] = "error"
  files = ['non_literal.pp', 'non_literal_2.pp'].map {|f| File.join(code_dir, f) }
  result = Puppet::InfoService.classes_per_environment({'production' => files })
+ expect(@logs).to have_matching_log_with_source(/The parameter '\$bad_int' must be a literal type, not a Puppet::Pops::Model::AccessExpression/, "#{code_dir}/non_literal.pp", 1, 37)
+ expect(@logs).to have_matching_log_with_source(/The parameter '\$double_brackets' must be a literal type, not a Puppet::Pops::Model::AccessExpression/, "#{code_dir}/non_literal_2.pp", 1, 44)
+
  expect(result).to eq({
  "production"=>{
  "#{code_dir}/non_literal.pp" =>
- {:error=> "The parameter '$bad_int' must be a literal type, not a Puppet::Pops::Model::AccessExpression (file: #{code_dir}/non_literal.pp, line: 1, column: 37)"},
+ {:error=> "The parameter '\$bad_int' is invalid: The expression <1-3> is not a valid type specification."},
  "#{code_dir}/non_literal_2.pp" =>
- {:error=> "The parameter '$double_brackets' must be a literal type, not a Puppet::Pops::Model::AccessExpression (file: #{code_dir}/non_literal_2.pp, line: 1, column: 44)"}
+ {:error=> "The parameter '\$double_brackets' is invalid: The expression <Optional[[String]]> is not a valid type specification."}
  } # end production env
  })
  end

spec/unit/pops/evaluator/literal_evaluator_spec.rb

@@ -21,9 +21,9 @@
  'Integer[-1]' => [-1],
  'Integer[-5, -1]' => [-5, -1],
  'Integer[-5, 5]' => [-5, 5],
- # we can't actually represent MIN_INTEGER because it's glexed as
+ # we can't actually represent MIN_INTEGER below, because it's lexed as
  # UnaryMinusExpression containing a positive LiteralInteger and the integer
- # must be <= MAX_INTEGER
+ # must be <= MAX_INTEGER. Therefore, the effective minimum is one greater.
  "Integer[#{Puppet::Pops::MIN_INTEGER + 1}]" => [-0x7FFFFFFFFFFFFFFF],
  "Integer[0, #{Puppet::Pops::MAX_INTEGER}]" => [0, 0x7FFFFFFFFFFFFFFF],
  'Integer[0, default]' => [0, :default],

spec/unit/pops/validator/validator_spec.rb

@@ -212,13 +212,6 @@ def with_environment(environment, env_params = {})
  expect(acceptor).to have_issue(Puppet::Pops::Issues::ILLEGAL_TOP_CONSTRUCT_LOCATION)
  end
  end
-
- it 'produces a warning for non-literal class parameters' do
- acceptor = validate(parse('class test(Integer[2-1] $port) {}'))
- expect(acceptor.warning_count).to eql(1)
- expect(acceptor.error_count).to eql(0)
- expect(acceptor).to have_issue(Puppet::Pops::Issues::ILLEGAL_NONLITERAL_PARAMETER_TYPE)
- end
  end
 
  context 'with --strict set to error' do
@@ -270,13 +263,6 @@ def with_environment(environment, env_params = {})
  expect(acceptor).to have_issue(Puppet::Pops::Issues::ILLEGAL_TOP_CONSTRUCT_LOCATION)
  end
  end
-
- it 'produces an error for non-literal class parameters' do
- acceptor = validate(parse('class test(Integer[2-1] $port) {}'))
- expect(acceptor.warning_count).to eql(0)
- expect(acceptor.error_count).to eql(1)
- expect(acceptor).to have_issue(Puppet::Pops::Issues::ILLEGAL_NONLITERAL_PARAMETER_TYPE)
- end
  end
 
  context 'with --strict set to off' do
@@ -307,13 +293,6 @@ def with_environment(environment, env_params = {})
  expect(acceptor).to have_issue(Puppet::Pops::Issues::ILLEGAL_TOP_CONSTRUCT_LOCATION)
  end
  end
-
- it 'does not produce an error or warning for non-literal class parameters' do
- acceptor = validate(parse('class test(Integer[2-1] $port) {}'))
- expect(acceptor.warning_count).to eql(0)
- expect(acceptor.error_count).to eql(0)
- expect(acceptor).to_not have_issue(Puppet::Pops::Issues::ILLEGAL_NONLITERAL_PARAMETER_TYPE)
- end
  end
 
  context 'irrespective of --strict' do
@@ -344,6 +323,14 @@ def with_environment(environment, env_params = {})
  expect(acceptor.error_count).to eql(1)
  expect(acceptor).to have_issue(Puppet::Pops::Issues::CLASS_NOT_VIRTUALIZABLE)
  end
+
+ it 'produces a deprecation warning for non-literal class parameters' do
+ acceptor = validate(parse('class test(Integer[2-1] $port) {}'))
+ expect(deprecation_count(acceptor)).to eql(1)
+ expect(acceptor.warning_count).to eql(1)
+ expect(acceptor.error_count).to eql(0)
+ expect(acceptor).to have_issue(Puppet::Pops::Issues::ILLEGAL_NONLITERAL_PARAMETER_TYPE)
+ end
  end
 
  context 'with --tasks set' do