--

PiperOrigin-RevId: 146926312
MOS_MIGRATED_REVID=146926312

3 files changed, 41 insertions, 13 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
index 060fa0894c..fd1f95f02e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java
@@ -302,10 +302,7 @@ public class ProtoCompileActionBuilder {
       // Note: the %s in the line below is used by proto-compiler. That is, the string we create
       // here should have a literal %s in it.
       result.add(
-          "--direct_dependencies_violation_msg=%s is imported, "
-              + "but "
-              + ruleContext.getLabel().getCanonicalForm()
-              + " doesn't directly depend on a proto_library that 'srcs' it.");
+          createStrictProtoDepsViolationErrorMessage(ruleContext.getLabel().getCanonicalForm()));
     }
 
     for (Artifact src : supportData.getDirectProtoSources()) {
@@ -576,13 +573,7 @@ public class ProtoCompileActionBuilder {
     cmdLine.add(new ProtoCommandLineArgv(protosInDirectDeps, transitiveSources));
 
     if (protosInDirectDeps != null) {
-      // Note: the %s in the line below is used by proto-compiler. That is, the string we create
-      // here should have a literal %s in it.
-      cmdLine.add(
-          "--direct_dependencies_violation_msg=%s is imported, "
-              + "but "
-              + ruleLabel
-              + " doesn't directly depend on a proto_library that 'srcs' it.");
+      cmdLine.add(createStrictProtoDepsViolationErrorMessage(ruleLabel));
     }
 
     for (Artifact src : protosToCompile) {
@@ -596,6 +587,14 @@ public class ProtoCompileActionBuilder {
     return cmdLine.build();
   }
 
+  @SuppressWarnings("FormatString") // Errorprone complains that there's no '%s' in the format
+  // string, but it's actually in MESSAGE.
+  @VisibleForTesting
+  public static String createStrictProtoDepsViolationErrorMessage(String ruleLabel) {
+    return "--direct_dependencies_violation_msg="
+        + String.format(StrictProtoDepsViolationMessage.MESSAGE, ruleLabel);
+  }
+
   /**
    * Describes a toolchain and the value to replace for a $(OUT) that might appear in its
    * commandLine() (e.g., "bazel-out/foo.srcjar").
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/StrictProtoDepsViolationMessage.java b/src/main/java/com/google/devtools/build/lib/rules/proto/StrictProtoDepsViolationMessage.java
new file mode 100644
index 0000000000..6d6d03844d
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/StrictProtoDepsViolationMessage.java
@@ -0,0 +1,29 @@
+// 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.build.lib.rules.proto;
+
+/**
+ * This class is used in ProtoCompileActionBuilder to generate an error message that's displayed
+ * when a strict proto deps violation occurs.
+ *
+ * <p>%1$s is replaced with the label of the proto_library rule that's currently being built.
+ *
+ * <p>%%s is replaced with the literal "%s", which is passed to the proto-compiler, which replaces
+ * it with the .proto file that violates strict proto deps.
+ */
+public class StrictProtoDepsViolationMessage {
+  static final String MESSAGE =
+      "%%s is imported, but %1$s doesn't directly depend on a proto_library that 'srcs' it.";
+}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
index c5ea553cbe..2bb3800eb6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.proto;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER;
 import static com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.createCommandLineFromToolchains;
+import static com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.createStrictProtoDepsViolationErrorMessage;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 
@@ -139,8 +140,7 @@ public class ProtoCompileActionBuilderTest {
             "-Iimport1.proto=import1.proto",
             "-Iimport2.proto=import2.proto",
             "--direct_dependencies=import1.proto",
-            "--direct_dependencies_violation_msg=%s is imported, "
-                + "but //foo:bar doesn't directly depend on a proto_library that 'srcs' it.",
+            createStrictProtoDepsViolationErrorMessage("//foo:bar"),
             "source_file.proto")
         .inOrder();
   }