diff --git a/configs/compat_shim.go b/configs/compat_shim.go index e594ebd40..79360201e 100644 --- a/configs/compat_shim.go +++ b/configs/compat_shim.go @@ -69,28 +69,21 @@ func shimTraversalInString(expr hcl.Expression, wantKeyword bool) (hcl.Expressio ) diags = append(diags, tDiags...) - // For initial release our deprecation warnings are disabled to allow - // a period where modules can be compatible with both old and new - // conventions. - // FIXME: Re-enable these deprecation warnings in a release prior to - // Terraform 0.13 and then remove the shims altogether for 0.13. - /* - if wantKeyword { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "Quoted keywords are deprecated", - Detail: "In this context, keywords are expected literally rather than in quotes. Previous versions of Terraform required quotes, but that usage is now deprecated. Remove the quotes surrounding this keyword to silence this warning.", - Subject: &srcRange, - }) - } else { - diags = append(diags, &hcl.Diagnostic{ - Severity: hcl.DiagWarning, - Summary: "Quoted references are deprecated", - Detail: "In this context, references are expected literally rather than in quotes. Previous versions of Terraform required quotes, but that usage is now deprecated. Remove the quotes surrounding this reference to silence this warning.", - Subject: &srcRange, - }) - } - */ + if wantKeyword { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Quoted keywords are deprecated", + Detail: "In this context, keywords are expected literally rather than in quotes. Terraform 0.11 and earlier required quotes, but quoted keywords are now deprecated and will be removed in a future version of Terraform. Remove the quotes surrounding this keyword to silence this warning.", + Subject: &srcRange, + }) + } else { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagWarning, + Summary: "Quoted references are deprecated", + Detail: "In this context, references are expected literally rather than in quotes. Terraform 0.11 and earlier required quotes, but quoted references are now deprecated and will be removed in a future version of Terraform. Remove the quotes surrounding this reference to silence this warning.", + Subject: &srcRange, + }) + } return &hclsyntax.ScopeTraversalExpr{ Traversal: traversal, diff --git a/configs/parser_config_test.go b/configs/parser_config_test.go index 22272cf07..13d90fed6 100644 --- a/configs/parser_config_test.go +++ b/configs/parser_config_test.go @@ -1,10 +1,15 @@ package configs import ( + "bufio" + "bytes" "io/ioutil" "path/filepath" + "strings" "testing" + "github.com/google/go-cmp/cmp" + "github.com/hashicorp/hcl/v2" ) @@ -34,8 +39,8 @@ func TestParserLoadConfigFileSuccess(t *testing.T) { }) _, diags := parser.LoadConfigFile(name) - if diags.HasErrors() { - t.Errorf("unexpected error diagnostics") + if len(diags) != 0 { + t.Errorf("unexpected diagnostics") for _, diag := range diags { t.Logf("- %s", diag) } @@ -124,16 +129,6 @@ func TestParserLoadConfigFileFailureMessages(t *testing.T) { hcl.DiagError, "Unsuitable value type", }, - { - "valid-files/resources-ignorechanges-all-legacy.tf", - hcl.DiagWarning, - "Deprecated ignore_changes wildcard", - }, - { - "valid-files/resources-ignorechanges-all-legacy.tf.json", - hcl.DiagWarning, - "Deprecated ignore_changes wildcard", - }, } for _, test := range tests { @@ -164,3 +159,68 @@ func TestParserLoadConfigFileFailureMessages(t *testing.T) { }) } } + +// TestParseLoadConfigFileWarning is a test that verifies files from +// testdata/warning-files produce particular warnings. +// +// This test does not verify that reading these files produces the correct +// file element contents in spite of those warnings. More detailed assertions +// may be made on some subset of these configuration files in other tests. +func TestParserLoadConfigFileWarning(t *testing.T) { + files, err := ioutil.ReadDir("testdata/warning-files") + if err != nil { + t.Fatal(err) + } + + for _, info := range files { + name := info.Name() + t.Run(name, func(t *testing.T) { + src, err := ioutil.ReadFile(filepath.Join("testdata/warning-files", name)) + if err != nil { + t.Fatal(err) + } + + // First we'll scan the file to see what warnings are expected. + // That's declared inside the files themselves by using the + // string "WARNING: " somewhere on each line that is expected + // to produce a warning, followed by the expected warning summary + // text. A single-line comment (with #) is the main way to do that. + const marker = "WARNING: " + sc := bufio.NewScanner(bytes.NewReader(src)) + wantWarnings := make(map[int]string) + lineNum := 1 + for sc.Scan() { + lineText := sc.Text() + if idx := strings.Index(lineText, marker); idx != -1 { + summaryText := lineText[idx+len(marker):] + wantWarnings[lineNum] = summaryText + } + lineNum++ + } + + parser := testParser(map[string]string{ + name: string(src), + }) + + _, diags := parser.LoadConfigFile(name) + if diags.HasErrors() { + t.Errorf("unexpected error diagnostics") + for _, diag := range diags { + t.Logf("- %s", diag) + } + } + + gotWarnings := make(map[int]string) + for _, diag := range diags { + if diag.Severity != hcl.DiagWarning || diag.Subject == nil { + continue + } + gotWarnings[diag.Subject.Start.Line] = diag.Summary + } + + if diff := cmp.Diff(wantWarnings, gotWarnings); diff != "" { + t.Errorf("wrong warnings\n%s", diff) + } + }) + } +} diff --git a/configs/testdata/valid-files/resources-dependson-quoted.tf b/configs/testdata/valid-files/resources-dependson-quoted.tf deleted file mode 100644 index 3bf188f19..000000000 --- a/configs/testdata/valid-files/resources-dependson-quoted.tf +++ /dev/null @@ -1,8 +0,0 @@ -resource "aws_security_group" "firewall" { -} - -resource "aws_instance" "web" { - depends_on = [ - "aws_security_group.firewall", - ] -} diff --git a/configs/testdata/valid-files/resources-ignorechanges-all-legacy.tf b/configs/testdata/valid-files/resources-ignorechanges-all-legacy.tf deleted file mode 100644 index 6b5e61a9c..000000000 --- a/configs/testdata/valid-files/resources-ignorechanges-all-legacy.tf +++ /dev/null @@ -1,5 +0,0 @@ -resource "aws_instance" "web" { - lifecycle { - ignore_changes = ["*"] - } -} diff --git a/configs/testdata/valid-files/resources-ignorechanges-all-legacy.tf.json b/configs/testdata/valid-files/resources-ignorechanges-all-legacy.tf.json deleted file mode 100644 index 5502dcd50..000000000 --- a/configs/testdata/valid-files/resources-ignorechanges-all-legacy.tf.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "resource": { - "aws_instance": { - "web": { - "lifecycle": { - "ignore_changes": ["*"] - } - } - } - } -} diff --git a/configs/testdata/valid-files/resources-ignorechanges-quoted.tf b/configs/testdata/valid-files/resources-ignorechanges-quoted.tf deleted file mode 100644 index cba5be59d..000000000 --- a/configs/testdata/valid-files/resources-ignorechanges-quoted.tf +++ /dev/null @@ -1,7 +0,0 @@ -resource "aws_instance" "web" { - lifecycle { - ignore_changes = [ - "ami", - ] - } -} diff --git a/configs/testdata/valid-files/resources-provisioner-onfailure-quoted.tf b/configs/testdata/valid-files/resources-provisioner-onfailure-quoted.tf deleted file mode 100644 index dcec1eb08..000000000 --- a/configs/testdata/valid-files/resources-provisioner-onfailure-quoted.tf +++ /dev/null @@ -1,6 +0,0 @@ -resource "aws_security_group" "firewall" { - provisioner "local-exec" { - command = "echo hello" - on_failure = "continue" - } -} diff --git a/configs/testdata/valid-files/resources-provisioner-when-quoted.tf b/configs/testdata/valid-files/resources-provisioner-when-quoted.tf deleted file mode 100644 index 6a66b085f..000000000 --- a/configs/testdata/valid-files/resources-provisioner-when-quoted.tf +++ /dev/null @@ -1,6 +0,0 @@ -resource "aws_security_group" "firewall" { - provisioner "local-exec" { - command = "echo hello" - when = "destroy" - } -} diff --git a/configs/testdata/warning-files/depends_on.tf b/configs/testdata/warning-files/depends_on.tf new file mode 100644 index 000000000..17e1bf34a --- /dev/null +++ b/configs/testdata/warning-files/depends_on.tf @@ -0,0 +1,6 @@ +resource "null_resource" "a" { +} + +resource "null_resource" "b" { + depends_on = ["null_resource.a"] # WARNING: Quoted references are deprecated +} diff --git a/configs/testdata/warning-files/ignore_changes.tf b/configs/testdata/warning-files/ignore_changes.tf new file mode 100644 index 000000000..5678fe7bd --- /dev/null +++ b/configs/testdata/warning-files/ignore_changes.tf @@ -0,0 +1,11 @@ +resource "null_resource" "one" { + lifecycle { + ignore_changes = ["triggers"] # WARNING: Quoted references are deprecated + } +} + +resource "null_resource" "all" { + lifecycle { + ignore_changes = ["*"] # WARNING: Deprecated ignore_changes wildcard + } +} diff --git a/configs/testdata/warning-files/provider_ref.tf b/configs/testdata/warning-files/provider_ref.tf new file mode 100644 index 000000000..6f5525ed7 --- /dev/null +++ b/configs/testdata/warning-files/provider_ref.tf @@ -0,0 +1,7 @@ +provider "null" { + alias = "foo" +} + +resource "null_resource" "test" { + provider = "null.foo" # WARNING: Quoted references are deprecated +} diff --git a/configs/testdata/warning-files/provisioner_keyword.tf b/configs/testdata/warning-files/provisioner_keyword.tf new file mode 100644 index 000000000..61fe72bdd --- /dev/null +++ b/configs/testdata/warning-files/provisioner_keyword.tf @@ -0,0 +1,6 @@ +resource "null_resource" "a" { + provisioner "local-exec" { + when = "create" # WARNING: Quoted keywords are deprecated + on_failure = "fail" # WARNING: Quoted keywords are deprecated + } +}