Merge pull request #189 from danw/badphony

Add new check for non-phony targets that have no commands

6 files changed, 161 insertions, 1 deletions

diff --git a/dep.cc b/dep.cc
index 65687cf..a4eb2d2 100644
--- a/dep.cc
+++ b/dep.cc
@@ -790,6 +790,53 @@ class DepBuilder {
       n->order_onlys.push_back({input, c});
     }
 
+    // Block on werror_writable/werror_phony_looks_real, because otherwise we
+    // can't rely on is_phony being valid for this check.
+    if (!n->is_phony && n->cmds.empty() && g_flags.werror_writable &&
+        g_flags.werror_phony_looks_real) {
+      if (n->deps.empty() && n->order_onlys.empty()) {
+        if (g_flags.werror_real_no_cmds_or_deps) {
+          ERROR_LOC(
+              n->loc,
+              "*** target \"%s\" has no commands or deps that could create it",
+              output.c_str());
+        } else if (g_flags.warn_real_no_cmds_or_deps) {
+          WARN_LOC(n->loc,
+                   "warning: target \"%s\" has no commands or deps that could "
+                   "create it",
+                   output.c_str());
+        }
+      } else {
+        if (n->actual_inputs.size() == 1) {
+          if (g_flags.werror_real_no_cmds) {
+            ERROR_LOC(n->loc,
+                      "*** target \"%s\" has no commands. Should \"%s\" be "
+                      "using .KATI_IMPLICIT_OUTPUTS?",
+                      output.c_str(), n->actual_inputs[0].c_str());
+          } else if (g_flags.warn_real_no_cmds) {
+            WARN_LOC(n->loc,
+                     "warning: target \"%s\" has no commands. Should \"%s\" be "
+                     "using .KATI_IMPLICIT_OUTPUTS?",
+                     output.c_str(), n->actual_inputs[0].c_str());
+          }
+        } else {
+          if (g_flags.werror_real_no_cmds) {
+            ERROR_LOC(
+                n->loc,
+                "*** target \"%s\" has no commands that could create output "
+                "file. Is a dependency missing .KATI_IMPLICIT_OUTPUTS?",
+                output.c_str());
+          } else if (g_flags.warn_real_no_cmds) {
+            WARN_LOC(
+                n->loc,
+                "warning: target \"%s\" has no commands that could create "
+                "output file. Is a dependency missing .KATI_IMPLICIT_OUTPUTS?",
+                output.c_str());
+          }
+        }
+      }
+    }
+
     n->has_rule = true;
     n->is_default_target = first_rule_ == output;
     if (cur_rule_vars_->empty()) {
diff --git a/dep.h b/dep.h
index 259c017..963447b 100644
--- a/dep.h
+++ b/dep.h
@@ -39,7 +39,6 @@ struct DepNode {
   vector<Value*> cmds;
   vector<NamedDepNode> deps;
   vector<NamedDepNode> order_onlys;
-  vector<NamedDepNode> parents;
   bool has_rule;
   bool is_default_target;
   bool is_phony;
diff --git a/flags.cc b/flags.cc
index 148d59a..bcc1b4f 100644
--- a/flags.cc
+++ b/flags.cc
@@ -134,6 +134,16 @@ void Flags::Parse(int argc, char** argv) {
       werror_phony_looks_real = true;
     } else if (!strcmp(arg, "--werror_writable")) {
       werror_writable = true;
+    } else if (!strcmp(arg, "--warn_real_no_cmds_or_deps")) {
+      warn_real_no_cmds_or_deps = true;
+    } else if (!strcmp(arg, "--werror_real_no_cmds_or_deps")) {
+      warn_real_no_cmds_or_deps = true;
+      werror_real_no_cmds_or_deps = true;
+    } else if (!strcmp(arg, "--warn_real_no_cmds")) {
+      warn_real_no_cmds = true;
+    } else if (!strcmp(arg, "--werror_real_no_cmds")) {
+      warn_real_no_cmds = true;
+      werror_real_no_cmds = true;
     } else if (ParseCommandLineOptionWithArg("-j", argv, &i, &num_jobs_str)) {
       num_jobs = strtol(num_jobs_str, NULL, 10);
       if (num_jobs <= 0) {
diff --git a/flags.h b/flags.h
index ee16c4f..279e7fe 100644
--- a/flags.h
+++ b/flags.h
@@ -56,6 +56,10 @@ struct Flags {
   bool warn_phony_looks_real;
   bool werror_phony_looks_real;
   bool werror_writable;
+  bool warn_real_no_cmds_or_deps;
+  bool werror_real_no_cmds_or_deps;
+  bool warn_real_no_cmds;
+  bool werror_real_no_cmds;
   const char* default_pool;
   const char* goma_dir;
   const char* ignore_dirty_pattern;
diff --git a/testcase/real_no_cmds.sh b/testcase/real_no_cmds.sh
new file mode 100644
index 0000000..88066d7
--- /dev/null
+++ b/testcase/real_no_cmds.sh
@@ -0,0 +1,53 @@
+#!/bin/bash
+#
+# Copyright 2020 Google Inc. 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.
+
+set -u
+
+mk="$@"
+
+cat <<EOF > Makefile
+test: out/bad.baz out/good.baz
+	@echo "END"
+.PHONY: test
+
+out/bad.bar:
+	@echo bar >out/bad.bar
+	@echo baz >out/bad.baz
+
+out/bad.baz: out/bad.bar
+
+out/good.bar: .KATI_IMPLICIT_OUTPUTS := out/good.baz
+out/good.bar:
+	@echo bar >out/good.bar
+	@echo baz >out/good.baz
+EOF
+
+mkdir -p out
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't support these warnings, so write the expected output.
+  echo 'Makefile:9: warning: target "out/bad.baz" has no commands. Should "out/bad.bar" be using .KATI_IMPLICIT_OUTPUTS?'
+  echo 'END'
+else
+  ${mk} --werror_phony_looks_real --writable=out/ --werror_writable --warn_real_no_cmds 2>&1
+fi
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't support these warnings, so write the expected output.
+  echo 'Makefile:9: *** target "out/bad.baz" has no commands. Should "out/bad.bar" be using .KATI_IMPLICIT_OUTPUTS?'
+else
+  ${mk} --werror_phony_looks_real --writable=out/ --werror_writable --werror_real_no_cmds 2>&1
+fi
diff --git a/testcase/real_no_cmds_or_deps.sh b/testcase/real_no_cmds_or_deps.sh
new file mode 100644
index 0000000..b591803
--- /dev/null
+++ b/testcase/real_no_cmds_or_deps.sh
@@ -0,0 +1,47 @@
+#!/bin/bash
+#
+# Copyright 2020 Google Inc. 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.
+
+set -u
+
+mk="$@"
+
+cat <<EOF > Makefile
+test: out/bad out/good
+	@echo "END"
+.PHONY: test
+
+out/bad:
+
+out/good:
+	@echo bar >out/good
+EOF
+
+mkdir -p out
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't support these warnings, so write the expected output.
+  echo 'Makefile:5: warning: target "out/bad" has no commands or deps that could create it'
+  echo 'END'
+else
+  ${mk} --werror_phony_looks_real --writable=out/ --werror_writable --warn_real_no_cmds_or_deps 2>&1
+fi
+
+if echo "${mk}" | grep -qv "kati"; then
+  # Make doesn't support these warnings, so write the expected output.
+  echo 'Makefile:5: *** target "out/bad" has no commands or deps that could create it'
+else
+  ${mk} --werror_phony_looks_real --writable=out/ --werror_writable --werror_real_no_cmds_or_deps 2>&1
+fi