diff options
author | fzaiser <fzaiser@google.com> | 2017-08-28 13:39:31 +0200 |
---|---|---|
committer | Vladimir Moskva <vladmos@google.com> | 2017-08-28 16:09:52 +0200 |
commit | 631126a89e204187b6b54038342464559754004f (patch) | |
tree | 0010301781a1c3968f100046a8fd2d71b0da04d8 /src/tools/skylark/javatests | |
parent | 27db2d8f0923751f99ea824096188153d49839ea (diff) |
Add a lint for missing return statements
This lint checks that if a functions returns a value in some execution
paths, it does so in all execution paths.
RELNOTES: None
PiperOrigin-RevId: 166688783
Diffstat (limited to 'src/tools/skylark/javatests')
-rw-r--r-- | src/tools/skylark/javatests/com/google/devtools/skylark/skylint/ControlFlowCheckerTest.java | 220 |
1 files changed, 220 insertions, 0 deletions
diff --git a/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/ControlFlowCheckerTest.java b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/ControlFlowCheckerTest.java new file mode 100644 index 0000000000..7e7addd8a2 --- /dev/null +++ b/src/tools/skylark/javatests/com/google/devtools/skylark/skylint/ControlFlowCheckerTest.java @@ -0,0 +1,220 @@ +// Copyright 2017 The Bazel Authors. All rights reserved. +// +// 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 com.google.devtools.skylark.skylint; + +import com.google.common.truth.Truth; +import com.google.devtools.build.lib.syntax.BuildFileAST; +import java.util.List; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class ControlFlowCheckerTest { + private static List<Issue> findIssues(String... lines) { + String content = String.join("\n", lines); + BuildFileAST ast = + BuildFileAST.parseSkylarkString( + event -> { + throw new IllegalArgumentException(event.getMessage()); + }, + content); + return ControlFlowChecker.check(ast); + } + + @Test + public void testIfElseReturnMissing() throws Exception { + Truth.assertThat( + findIssues( + "def some_function(x):", + " if x:", + " print('foo')", + " else:", + " return x") + .toString()) + .contains("some but not all execution paths of 'some_function' return a value"); + } + + @Test + public void testIfElseReturnValueMissing() throws Exception { + String messages = + findIssues( + "def some_function(x):", + " if x:", + " return x", + " else:", + " return # missing value") + .toString(); + Truth.assertThat(messages) + .contains("some but not all execution paths of 'some_function' return a value"); + Truth.assertThat(messages).contains(":5:5: return value missing"); + } + + @Test + public void testIfElifElseReturnMissing() throws Exception { + Truth.assertThat( + findIssues( + "def f(x):", + " if x:", + " return x", + " elif not x:", + " pass", + " else:", + " return not x") + .toString()) + .contains("some but not all execution paths of 'f' return a value"); + } + + @Test + public void testNestedIfElseReturnMissing() throws Exception { + Truth.assertThat( + findIssues( + "def f(x, y):", + " if x:", + " if y:", + " return y", + " else:", + " print('foo')", + " else:", + " return x") + .toString()) + .contains("some but not all execution paths of 'f' return a value"); + } + + @Test + public void testElseBranchMissing() throws Exception { + Truth.assertThat( + findIssues( + "def some_function(x):", + " if x:", + " return x", + " elif not x:", + " return not x") + .toString()) + .contains("some but not all execution paths of 'some_function' return a value"); + } + + @Test + public void testIfAndFallOffEnd() throws Exception { + Truth.assertThat( + findIssues( + "def some_function(x):", + " if x:", + " return x", + " print('foo')", + " # return missing here") + .toString()) + .contains("some but not all execution paths of 'some_function' return a value"); + } + + @Test + public void testAlwaysReturnButSometimesWithoutValue() throws Exception { + String messages = + findIssues( + "def some_function(x):", + " if x:", + " return # returns without value here", + " return x") + .toString(); + Truth.assertThat(messages) + .contains("some but not all execution paths of 'some_function' return a value"); + Truth.assertThat(messages).contains(":3:5: return value missing"); + } + + @Test + public void testIfBeforeReturn() throws Exception { + Truth.assertThat( + findIssues( + "def f(x, y):", + " if x:", + " return x", + " elif not y:", + " print('foo')", + " print('bar')", + " return y")) + .isEmpty(); + } + + @Test + public void testReturnInAllBranches() throws Exception { + Truth.assertThat( + findIssues( + "def f(x, y):", + " if x:", + " return x", + " elif not y:", + " return None", + " else:", + " return y")) + .isEmpty(); + } + + @Test + public void testReturnInNestedIf() throws Exception { + Truth.assertThat( + findIssues( + "def f(x,y):", + " if x:", + " if y:", + " return y", + " else:", + " return not y", + " else:", + " return not x")) + .isEmpty(); + } + + @Test + public void testIfStatementSequence() throws Exception { + Truth.assertThat( + findIssues( + "def f(x,y):", + " if x:", + " print('foo')", + " else:", + " return x", + " print('bar')", + " if y:", + " return x", + " else:", + " return y")) + .isEmpty(); + Truth.assertThat( + findIssues( + "def f(x,y):", + " if x:", + " return x", + " else:", + " return x", + " # from now on everything's unreachable", + " print('bar')", + " if y:", + " return x", + " # no else branch but doesn't matter since it's unreachable")) + .isEmpty(); + } + + @Test + public void testCallToFail() throws Exception { + Truth.assertThat( + findIssues( + "def some_function_name(x):", + " if x:", + " fail('bar')", + " else:", + " return x")) + .isEmpty(); + } +} |