Merge pull request #383 from onflow/bastian/9-unused-result-analyzer

lint/analyzer.go

@@ -28,8 +28,8 @@ import (
 const (
 	ReplacementCategory     = "replacement-hint"
 	RemovalCategory         = "removal-hint"
-	UpdateCategory          = "update recommended"
 	UnnecessaryCastCategory = "unnecessary-cast-hint"
+	UnusedResultCategory    = "unused-result-hint"
 	DeprecatedCategory      = "deprecated"
 	CadenceV1Category       = "cadence-v1"
 )

lint/analyzers_test.go

@@ -36,24 +36,38 @@ func testAnalyzers(t *testing.T, code string, analyzers ...*analysis.Analyzer) [
 	return testAnalyzersAdvanced(t, code, nil, analyzers...)
 }
 
-func testAnalyzersWithCheckerError(t *testing.T, code string, analyzers ...*analysis.Analyzer) ([]analysis.Diagnostic, *sema.CheckerError) {
+func testAnalyzersWithCheckerError(
+	t *testing.T,
+	code string,
+	analyzers ...*analysis.Analyzer,
+) ([]analysis.Diagnostic, *sema.CheckerError) {
 	var checkerErr *sema.CheckerError
-	diagnostics := testAnalyzersAdvanced(t, code, func(config *analysis.Config) {
-		config.HandleCheckerError = func(err analysis.ParsingCheckingError, checker *sema.Checker) error {
-			require.NotNil(t, checker)
-			require.Equal(t, err.ImportLocation(), testLocation)
-
-			require.ErrorAs(t, err, &checkerErr)
-			require.Len(t, checkerErr.Errors, 1)
-			return nil
-		}
-	}, analyzers...)
+	diagnostics := testAnalyzersAdvanced(
+		t,
+		code,
+		func(config *analysis.Config) {
+			config.HandleCheckerError = func(err analysis.ParsingCheckingError, checker *sema.Checker) error {
+				require.NotNil(t, checker)
+				require.Equal(t, err.ImportLocation(), testLocation)
+
+				require.ErrorAs(t, err, &checkerErr)
+				require.Len(t, checkerErr.Errors, 1)
+				return nil
+			}
+		},
+		analyzers...,
+	)
 
 	require.NotNil(t, checkerErr)
 	return diagnostics, checkerErr
 }
 
-func testAnalyzersAdvanced(t *testing.T, code string, setCustomConfigOptions func(config *analysis.Config), analyzers ...*analysis.Analyzer) []analysis.Diagnostic {
+func testAnalyzersAdvanced(
+	t *testing.T,
+	code string,
+	setCustomConfigOptions func(config *analysis.Config),
+	analyzers ...*analysis.Analyzer,
+) []analysis.Diagnostic {
 
 	config := analysis.NewSimpleConfig(
 		lint.LoadMode,

lint/unused_result_analyzer.go

@@ -0,0 +1,82 @@
+/*
+ * Cadence-lint - The Cadence linter
+ *
+ * Copyright 2019-2022 Dapper Labs, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package lint
+
+import (
+	"github.com/onflow/cadence/runtime/ast"
+	"github.com/onflow/cadence/runtime/sema"
+	"github.com/onflow/cadence/tools/analysis"
+)
+
+var UnusedResultAnalyzer = (func() *analysis.Analyzer {
+
+	elementFilter := []ast.Element{
+		(*ast.ExpressionStatement)(nil),
+	}
+
+	return &analysis.Analyzer{
+		Description: "Detects unused results of expressions",
+		Requires: []*analysis.Analyzer{
+			analysis.InspectorAnalyzer,
+		},
+		Run: func(pass *analysis.Pass) interface{} {
+			inspector := pass.ResultOf[analysis.InspectorAnalyzer].(*ast.Inspector)
+
+			program := pass.Program
+			location := program.Location
+			elaboration := program.Checker.Elaboration
+			report := pass.Report
+
+			inspector.Preorder(
+				elementFilter,
+				func(element ast.Element) {
+
+					expressionStatement, ok := element.(*ast.ExpressionStatement)
+					if !ok {
+						return
+					}
+
+					ty := elaboration.ExpressionTypes(expressionStatement.Expression).ActualType
+					switch ty {
+					case nil, sema.VoidType, sema.NeverType:
+						// NO-OP
+					default:
+						report(
+							analysis.Diagnostic{
+								Location: location,
+								Range:    ast.NewRangeFromPositioned(nil, element),
+								Category: UnusedResultCategory,
+								Message:  "unused result",
+							},
+						)
+					}
+				},
+			)
+
+			return nil
+		},
+	}
+})()
+
+func init() {
+	RegisterAnalyzer(
+		"unused-result",
+		UnusedResultAnalyzer,
+	)
+}

lint/unused_result_analyzer_test.go

@@ -0,0 +1,173 @@
+/*
+ * Cadence-lint - The Cadence linter
+ *
+ * Copyright 2019-2022 Dapper Labs, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package lint_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/onflow/cadence/runtime/ast"
+	"github.com/onflow/cadence/tools/analysis"
+
+	"github.com/onflow/cadence-tools/lint"
+)
+
+func TestUnusedResultAnalyzer(t *testing.T) {
+
+	t.Parallel()
+
+	t.Run("binary expression", func(t *testing.T) {
+
+		t.Parallel()
+
+		diagnostics := testAnalyzers(t,
+			`
+              access(all) fun test() {
+                  let x = 3
+                  x + 1
+              }
+            `,
+			lint.UnusedResultAnalyzer,
+		)
+
+		require.Equal(
+			t,
+			[]analysis.Diagnostic{
+				{
+					Range: ast.Range{
+						StartPos: ast.Position{Offset: 86, Line: 4, Column: 18},
+						EndPos:   ast.Position{Offset: 90, Line: 4, Column: 22},
+					},
+					Location: testLocation,
+					Category: lint.UnusedResultCategory,
+					Message:  "unused result",
+				},
+			},
+			diagnostics,
+		)
+	})
+
+	t.Run("non-void member function", func(t *testing.T) {
+
+		t.Parallel()
+
+		diagnostics := testAnalyzers(t,
+			`
+              access(all) fun test() {
+                  let string = "hello"
+                  string.concat("world")
+              }
+            `,
+			lint.UnusedResultAnalyzer,
+		)
+
+		require.Equal(
+			t,
+			[]analysis.Diagnostic{
+				{
+					Range: ast.Range{
+						StartPos: ast.Position{Offset: 97, Line: 4, Column: 18},
+						EndPos:   ast.Position{Offset: 118, Line: 4, Column: 39},
+					},
+					Location: testLocation,
+					Category: lint.UnusedResultCategory,
+					Message:  "unused result",
+				},
+			},
+			diagnostics,
+		)
+	})
+
+	t.Run("non-void function", func(t *testing.T) {
+
+		t.Parallel()
+
+		diagnostics := testAnalyzers(t,
+			`
+              access(all) fun answer(): Int {
+                  return 42
+              }
+
+              access(all) fun test() {
+                  answer()
+              }
+            `,
+			lint.UnusedResultAnalyzer,
+		)
+
+		require.Equal(
+			t,
+			[]analysis.Diagnostic{
+				{
+					Range: ast.Range{
+						StartPos: ast.Position{Offset: 149, Line: 7, Column: 18},
+						EndPos:   ast.Position{Offset: 156, Line: 7, Column: 25},
+					},
+					Location: testLocation,
+					Category: lint.UnusedResultCategory,
+					Message:  "unused result",
+				},
+			},
+			diagnostics,
+		)
+	})
+
+	t.Run("never function", func(t *testing.T) {
+
+		t.Parallel()
+
+		diagnostics := testAnalyzers(t,
+			`
+              access(all) fun test() {
+                  panic("test")
+              }
+            `,
+			lint.UnusedResultAnalyzer,
+		)
+
+		require.Equal(
+			t,
+			[]analysis.Diagnostic(nil),
+			diagnostics,
+		)
+	})
+
+	t.Run("void function", func(t *testing.T) {
+
+		t.Parallel()
+
+		diagnostics := testAnalyzers(t,
+			`
+              access(all) fun nothing() {}
+
+              access(all) fun test() {
+                  nothing()
+              }
+            `,
+			lint.UnusedResultAnalyzer,
+		)
+
+		require.Equal(
+			t,
+			[]analysis.Diagnostic(nil),
+			diagnostics,
+		)
+	})
+}