Merge pull request #2290 from onflow/holyfuchs/lint-warnings-as-errors

add --warnings-as-errors flag to flow cadence lint

Author: chasefleming

internal/cadence/lint.go

@@ -39,7 +39,9 @@ import (
 	"github.com/onflow/flow-cli/internal/util"
 )
 
-type lintFlagsCollection struct{}
+type lintFlagsCollection struct {
+	WarningsAsErrors bool `default:"false" flag:"warnings-as-errors" info:"Treat warnings as errors"`
+}
 
 type fileResult struct {
 	FilePath    string
@@ -98,7 +100,7 @@ func lint(
 		filePaths = args
 	}
 
-	result, err := lintFiles(state, filePaths...)
+	result, err := lintFiles(state, lintFlags.WarningsAsErrors, filePaths...)
 	if err != nil {
 		return nil, err
 	}
@@ -108,6 +110,7 @@ func lint(
 
 func lintFiles(
 	state *flowkit.State,
+	warningsAsErrors bool,
 	filePaths ...string,
 ) (
 	*lintResult,
@@ -140,14 +143,18 @@ func lintFiles(
 			Diagnostics: diagnostics,
 		})
 
-		// Set the exitCode to 1 if any of the diagnostics are error-level
-		// In the future, this may be configurable
+		// Set the exitCode to 1 if any of the diagnostics are error-level,
+		// or warning-level when warningsAsErrors is set.
 		for _, diagnostic := range diagnostics {
 			severity := getDiagnosticSeverity(diagnostic)
 			if severity == errorSeverity {
 				exitCode = 1
 				break
 			}
+			if severity == warningSeverity && warningsAsErrors {
+				exitCode = 1
+				break
+			}
 		}
 	}
 

internal/cadence/lint_test.go

@@ -49,7 +49,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "NoError.cdc")
+		results, err := lintFiles(state, false, "NoError.cdc")
 		require.NoError(t, err)
 
 		require.Equal(t,
@@ -71,7 +71,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "foo/WithImports.cdc")
+		results, err := lintFiles(state, false, "foo/WithImports.cdc")
 		require.NoError(t, err)
 
 		// Should not have results for imported file, only for the file being linted
@@ -94,7 +94,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "NoError.cdc", "foo/WithImports.cdc")
+		results, err := lintFiles(state, false, "NoError.cdc", "foo/WithImports.cdc")
 		require.NoError(t, err)
 
 		require.Equal(t,
@@ -120,7 +120,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "LintWarning.cdc")
+		results, err := lintFiles(state, false, "LintWarning.cdc")
 		require.NoError(t, err)
 
 		require.Equal(t,
@@ -147,12 +147,26 @@ func Test_Lint(t *testing.T) {
 		)
 	})
 
+	t.Run("warnings as errors: exits 0 without flag, exits 1 with flag", func(t *testing.T) {
+		t.Parallel()
+
+		state := setupMockState(t)
+
+		results, err := lintFiles(state, false, "LintWarning.cdc")
+		require.NoError(t, err)
+		require.Equal(t, 0, results.exitCode)
+
+		results, err = lintFiles(state, true, "LintWarning.cdc")
+		require.NoError(t, err)
+		require.Equal(t, 1, results.exitCode)
+	})
+
 	t.Run("lints file with error", func(t *testing.T) {
 		t.Parallel()
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "LintError.cdc")
+		results, err := lintFiles(state, false, "LintError.cdc")
 		require.NoError(t, err)
 
 		require.Equal(t,
@@ -194,7 +208,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "ReplacementHint.cdc")
+		results, err := lintFiles(state, false, "ReplacementHint.cdc")
 		require.NoError(t, err)
 
 		require.Len(t, results.Results, 1)
@@ -221,7 +235,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "WithFlowkitImport.cdc")
+		results, err := lintFiles(state, false, "WithFlowkitImport.cdc")
 		require.NoError(t, err)
 
 		require.Equal(t,
@@ -243,7 +257,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "StdlibImportsContract.cdc")
+		results, err := lintFiles(state, false, "StdlibImportsContract.cdc")
 		require.NoError(t, err)
 
 		// Expects an error because getAuthAccount is only available in scripts
@@ -277,7 +291,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "StdlibImportsTransaction.cdc")
+		results, err := lintFiles(state, false, "StdlibImportsTransaction.cdc")
 		require.NoError(t, err)
 
 		// Expects an error because getAuthAccount is only available in scripts
@@ -311,7 +325,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "StdlibImportsScript.cdc")
+		results, err := lintFiles(state, false, "StdlibImportsScript.cdc")
 		require.NoError(t, err)
 
 		require.Equal(t,
@@ -333,7 +347,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "StdlibImportsCrypto.cdc")
+		results, err := lintFiles(state, false, "StdlibImportsCrypto.cdc")
 		require.NoError(t, err)
 
 		require.Equal(t,
@@ -355,7 +369,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockState(t)
 
-		results, err := lintFiles(state, "TransactionImportingContractWithNestedImports.cdc")
+		results, err := lintFiles(state, false, "TransactionImportingContractWithNestedImports.cdc")
 		require.NoError(t, err)
 
 		require.Equal(t,
@@ -377,7 +391,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockStateWithAccountAccess(t)
 
-		results, err := lintFiles(state, "ContractA.cdc")
+		results, err := lintFiles(state, false, "ContractA.cdc")
 		require.NoError(t, err)
 
 		// Should have no errors since ContractA and ContractB are on same account
@@ -400,7 +414,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockStateWithAccountAccess(t)
 
-		results, err := lintFiles(state, "ContractC.cdc")
+		results, err := lintFiles(state, false, "ContractC.cdc")
 		require.NoError(t, err)
 
 		// Should have error since ContractC and ContractB are on different accounts
@@ -416,7 +430,7 @@ func Test_Lint(t *testing.T) {
 
 		state := setupMockStateWithDependencies(t)
 
-		results, err := lintFiles(state, "imports/testaddr/DepA.cdc")
+		results, err := lintFiles(state, false, "imports/testaddr/DepA.cdc")
 		require.NoError(t, err)
 
 		// Should have no errors since DepA and DepB are dependencies on same address
@@ -448,7 +462,7 @@ func Test_Lint(t *testing.T) {
 		require.NotNil(t, alias, "Alias should be automatically created from Source")
 		require.Equal(t, "dfc20aee650fcbdf", alias.Address.String(), "Alias address should match Source address")
 
-		results, err := lintFiles(state, "imports/testaddr/SourceA.cdc")
+		results, err := lintFiles(state, false, "imports/testaddr/SourceA.cdc")
 		require.NoError(t, err)
 
 		// Should have no errors since SourceA and SourceB have same Source.Address (converted to Aliases)