starlark: remove EvalException(Location) -- easy cases

This change removes the Location argument to new EvalException() if:
- location is null or BUILTIN;
- location is rule.getLocation(), if the code is only reached while
  doing rule analysis, as analysis failures always report the rule
  location.
- the exception is never thrown out of a Starlark call; that is,
  it is only used locally in Java code.

Trickier cases are left for a follow-up.

Also, use Starlark.errorf to construct error messages.

PiperOrigin-RevId: 325546962

src/main/java/com/google/devtools/build/lib/analysis/StarlarkProviderValidationUtil.java

@@ -18,24 +18,25 @@
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.Starlark;
 
 /** Utility class to validate results of executing Starlark rules and aspects. */
 public class StarlarkProviderValidationUtil {
   public static void validateArtifacts(RuleContext ruleContext) throws EvalException {
     ImmutableSet<Artifact> treeArtifactsConflictingWithFiles =
         ruleContext.getAnalysisEnvironment().getTreeArtifactsConflictingWithFiles();
     if (!treeArtifactsConflictingWithFiles.isEmpty()) {
-      throw new EvalException(
-          "The following directories were also declared as files:\n"
-              + artifactsDescription(treeArtifactsConflictingWithFiles));
+      throw Starlark.errorf(
+          "The following directories were also declared as files:\n%s",
+          artifactsDescription(treeArtifactsConflictingWithFiles));
     }
 
     ImmutableSet<Artifact> orphanArtifacts =
         ruleContext.getAnalysisEnvironment().getOrphanArtifacts();
     if (!orphanArtifacts.isEmpty()) {
-      throw new EvalException(
-          "The following files have no generating action:\n"
-              + artifactsDescription(orphanArtifacts));
+      throw Starlark.errorf(
+          "The following files have no generating action:\n%s",
+          artifactsDescription(orphanArtifacts));
     }
   }
 

src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java

@@ -73,7 +73,6 @@
 import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
 import com.google.devtools.build.lib.starlarkbuildapi.CommandLineArgsApi;
 import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.Location;
 import com.google.devtools.build.lib.syntax.Sequence;
 import com.google.devtools.build.lib.syntax.StarlarkList;
 import com.google.devtools.build.lib.util.DetailedExitCode;
@@ -262,8 +261,8 @@ public Sequence<CommandLineArgsApi> getStarlarkArgs() throws EvalException {
   public Sequence<String> getStarlarkArgv() throws EvalException {
     try {
       return StarlarkList.immutableCopyOf(getArguments());
-    } catch (CommandLineExpansionException exception) {
-      throw new EvalException(Location.BUILTIN, exception);
+    } catch (CommandLineExpansionException ex) {
+      throw new EvalException(ex);
     }
   }
 

src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java

@@ -466,11 +466,9 @@ public CommandLineArgsApi useParamsFile(String paramFileArg, Boolean useAlways)
         throws EvalException {
       Starlark.checkMutable(this);
       if (!SingleStringArgFormatter.isValid(paramFileArg)) {
-        throw new EvalException(
-            String.format(
-                "Invalid value for parameter \"param_file_arg\": "
-                    + "Expected string with a single \"%s\"",
-                paramFileArg));
+        throw Starlark.errorf(
+            "Invalid value for parameter \"param_file_arg\": Expected string with a single \"%s\"",
+            paramFileArg);
       }
       this.flagFormatString = paramFileArg;
       this.alwaysUseParamFile = useAlways;
@@ -489,7 +487,7 @@ public CommandLineArgsApi setParamFileFormat(String format) throws EvalException
           parameterFileType = ParameterFileType.UNQUOTED;
           break;
         default:
-          throw new EvalException(
+          throw Starlark.errorf(
               "Invalid value for parameter \"format\": Expected one of \"shell\", \"multiline\"");
       }
       this.parameterFileType = parameterFileType;

src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java

@@ -102,8 +102,7 @@ static Map<String, BuildOptions> applyAndValidate(
   private static void checkForBlacklistedOptions(StarlarkDefinedConfigTransition transition)
       throws EvalException {
     if (transition.getOutputs().contains("//command_line_option:define")) {
-      throw new EvalException(
-          transition.getLocationForErrorReporting(),
+      throw Starlark.errorf(
           "Starlark transition on --define not supported - try using build settings"
               + " (https://docs.bazel.build/skylark/config.html#user-defined-build-settings).");
     }
@@ -123,18 +122,14 @@ private static void validateFunctionOutputsMatchesDeclaredOutputs(
           Sets.newLinkedHashSet(starlarkTransition.getOutputs());
       for (String outputKey : transition.keySet()) {
         if (!remainingOutputs.remove(outputKey)) {
-          throw new EvalException(
-              starlarkTransition.getLocationForErrorReporting(),
-              String.format("transition function returned undeclared output '%s'", outputKey));
+          throw Starlark.errorf("transition function returned undeclared output '%s'", outputKey);
         }
       }
 
       if (!remainingOutputs.isEmpty()) {
-        throw new EvalException(
-            starlarkTransition.getLocationForErrorReporting(),
-            String.format(
-                "transition outputs [%s] were not defined by transition function",
-                Joiner.on(", ").join(remainingOutputs)));
+        throw Starlark.errorf(
+            "transition outputs [%s] were not defined by transition function",
+            Joiner.on(", ").join(remainingOutputs));
       }
     }
   }
@@ -213,11 +208,9 @@ static Dict<String, Object> buildSettings(
       }
 
       if (!remainingInputs.isEmpty()) {
-        throw new EvalException(
-            starlarkTransition.getLocationForErrorReporting(),
-            String.format(
-                "transition inputs [%s] do not correspond to valid settings",
-                Joiner.on(", ").join(remainingInputs)));
+        throw Starlark.errorf(
+            "transition inputs [%s] do not correspond to valid settings",
+            Joiner.on(", ").join(remainingInputs));
       }
 
       return dict;
@@ -266,11 +259,8 @@ private static BuildOptions applyTransition(
         }
         try {
           if (!optionInfoMap.containsKey(optionName)) {
-            throw new EvalException(
-                starlarkTransition.getLocationForErrorReporting(),
-                String.format(
-                    "transition output '%s' does not correspond to a valid setting",
-                    entry.getKey()));
+            throw Starlark.errorf(
+                "transition output '%s' does not correspond to a valid setting", entry.getKey());
           }
 
           OptionInfo optionInfo = optionInfoMap.get(optionName);
@@ -306,23 +296,19 @@ private static BuildOptions applyTransition(
           } else if (optionValue instanceof String) {
             convertedValue = def.getConverter().convert((String) optionValue);
           } else {
-            throw new EvalException(
-                starlarkTransition.getLocationForErrorReporting(),
-                "Invalid value type for option '" + optionName + "'");
+            throw Starlark.errorf("Invalid value type for option '%s'", optionName);
           }
           field.set(options, convertedValue);
           convertedNewValues.put(entry.getKey(), convertedValue);
         } catch (IllegalArgumentException e) {
-          throw new EvalException(
-              starlarkTransition.getLocationForErrorReporting(),
-              "IllegalArgumentError for option '" + optionName + "': " + e.getMessage());
+          throw Starlark.errorf(
+              "IllegalArgumentError for option '%s': %s", optionName, e.getMessage());
         } catch (IllegalAccessException e) {
           throw new RuntimeException(
               "IllegalAccess for option " + optionName + ": " + e.getMessage());
         } catch (OptionsParsingException e) {
-          throw new EvalException(
-              starlarkTransition.getLocationForErrorReporting(),
-              "OptionsParsingError for option '" + optionName + "': " + e.getMessage());
+          throw Starlark.errorf(
+              "OptionsParsingError for option '%s': %s", optionName, e.getMessage());
         }
       }
     }

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java

@@ -452,9 +452,9 @@ public void runShell(
       List<String> command = Sequence.cast(commandList, String.class, "command");
       builder.setShellCommand(command);
     } else {
-      throw new EvalException(
-          "expected string or list of strings for command instead of "
-              + Starlark.type(commandUnchecked));
+      throw Starlark.errorf(
+          "expected string or list of strings for command instead of %s",
+          Starlark.type(commandUnchecked));
     }
     if (argumentList.size() > 0) {
       // When we use a shell command, add an empty argument before other arguments.
@@ -493,9 +493,9 @@ private static void buildCommandLine(SpawnAction.Builder builder, Sequence<?> ar
         ParamFileInfo paramFileInfo = args.getParamFileInfo();
         builder.addCommandLine(args.build(), paramFileInfo);
       } else {
-        throw new EvalException(
-            "expected list of strings or ctx.actions.args() for arguments instead of "
-                + Starlark.type(value));
+        throw Starlark.errorf(
+            "expected list of strings or ctx.actions.args() for arguments instead of %s",
+            Starlark.type(value));
       }
     }
     if (!stringArgs.isEmpty()) {
@@ -566,11 +566,10 @@ private void registerStarlarkAction(
         } else if (toolUnchecked instanceof FilesToRunProvider) {
           builder.addTool((FilesToRunProvider) toolUnchecked);
         } else {
-          throw new EvalException(
-              "expected value of type 'File or FilesToRunProvider' for "
-                  + "a member of parameter 'tools' but got "
-                  + Starlark.type(toolUnchecked)
-                  + " instead");
+          throw Starlark.errorf(
+              "expected value of type 'File or FilesToRunProvider' for a member of parameter"
+                  + " 'tools' but got %s instead",
+              Starlark.type(toolUnchecked));
         }
       }
     } else {

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java

@@ -85,7 +85,7 @@ private static void setAllowedFileTypes(
           ImmutableList.copyOf(Sequence.cast(fileTypesObj, String.class, "allow_files argument"));
       builder.allowedFileTypes(FileType.of(arg));
     } else {
-      throw new EvalException(attr + " should be a boolean or a string list");
+      throw Starlark.errorf("%s should be a boolean or a string list", attr);
     }
   }
 
@@ -163,7 +163,7 @@ private static Attribute.Builder<?> createAttribute(
     if (containsNonNoneKey(arguments, EXECUTABLE_ARG) && (Boolean) arguments.get(EXECUTABLE_ARG)) {
       builder.setPropertyFlag("EXECUTABLE");
       if (!containsNonNoneKey(arguments, CONFIGURATION_ARG)) {
-        throw new EvalException(
+        throw Starlark.errorf(
             "cfg parameter is mandatory when executable=True is provided. Please see "
                 + "https://www.bazel.build/versions/master/docs/skylark/rules.html#configurations "
                 + "for more details.");
@@ -172,7 +172,7 @@ private static Attribute.Builder<?> createAttribute(
 
     if (containsNonNoneKey(arguments, ALLOW_FILES_ARG)
         && containsNonNoneKey(arguments, ALLOW_SINGLE_FILE_ARG)) {
-      throw new EvalException("Cannot specify both allow_files and allow_single_file");
+      throw Starlark.errorf("Cannot specify both allow_files and allow_single_file");
     }
 
     if (containsNonNoneKey(arguments, ALLOW_FILES_ARG)) {
@@ -219,7 +219,7 @@ && containsNonNoneKey(arguments, ALLOW_SINGLE_FILE_ARG)) {
               || trans instanceof TransitionFactory
               || trans instanceof StarlarkDefinedConfigTransition;
       if (isSplit && defaultValue instanceof StarlarkLateBoundDefault) {
-        throw new EvalException(
+        throw Starlark.errorf(
             "late-bound attributes must not have a split configuration transition");
       }
       if (trans.equals("host")) {
@@ -239,7 +239,7 @@ && containsNonNoneKey(arguments, ALLOW_SINGLE_FILE_ARG)) {
           builder.hasAnalysisTestTransition();
         } else {
           if (!thread.getSemantics().experimentalStarlarkConfigTransitions()) {
-            throw new EvalException(
+            throw Starlark.errorf(
                 "Starlark-defined transitions on rule attributes is experimental and disabled by "
                     + "default. This API is in development and subject to change at any time. Use "
                     + "--experimental_starlark_config_transitions to use this experimental API.");
@@ -249,7 +249,7 @@ && containsNonNoneKey(arguments, ALLOW_SINGLE_FILE_ARG)) {
         builder.cfg(new StarlarkAttributeTransitionProvider(starlarkDefinedTransition));
       } else if (!trans.equals("target")) {
         // TODO(b/121134880): update error message when starlark build configurations is ready.
-        throw new EvalException("cfg must be either 'host' or 'target'.");
+        throw Starlark.errorf("cfg must be either 'host' or 'target'.");
       }
     }
 
@@ -311,7 +311,7 @@ static ImmutableSet<StarlarkProviderIdentifier> getStarlarkProviderIdentifiers(S
       } else if (obj instanceof Provider) {
         Provider constructor = (Provider) obj;
         if (!constructor.isExported()) {
-          throw new EvalException(
+          throw Starlark.errorf(
               "Providers should be top-level values in extension files that define them.");
         }
         result.add(StarlarkProviderIdentifier.forKey(constructor.getKey()));
@@ -330,14 +330,12 @@ private static ImmutableList<ImmutableSet<StarlarkProviderIdentifier>> getProvid
 
     for (Object o : starlarkList) {
       if (!(o instanceof Sequence)) {
-        throw new EvalException(
-            String.format(errorMsg, PROVIDERS_ARG, "an element of type " + Starlark.type(o)));
+        throw Starlark.errorf(errorMsg, PROVIDERS_ARG, "an element of type " + Starlark.type(o));
       }
       for (Object value : (Sequence) o) {
         if (!isProvider(value)) {
-          throw new EvalException(
-              String.format(
-                  errorMsg, argumentName, "list with an element of type " + Starlark.type(value)));
+          throw Starlark.errorf(
+              errorMsg, argumentName, "list with an element of type " + Starlark.type(value));
         }
       }
       providersList.add(getStarlarkProviderIdentifiers((Sequence<?>) o));

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkErrorReporter.java

@@ -16,6 +16,7 @@
 import com.google.devtools.build.lib.analysis.RuleErrorConsumer;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.Starlark;
 
 /**
  * {@link RuleErrorConsumer} for Native implementations of Starlark APIs.
@@ -43,7 +44,7 @@ public void close() throws EvalException {
     try {
       assertNoErrors();
     } catch (RuleErrorException e) {
-      throw new EvalException("error occurred while evaluating builtin function", e);
+      throw Starlark.errorf("error occurred while evaluating builtin function: %s", e.getMessage());
     }
   }
 

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -323,7 +323,7 @@ public StarlarkCallable rule(
             new StarlarkCallbackHelper(
                 (StarlarkFunction) implicitOutputs, thread.getSemantics(), bazelContext);
         builder.setImplicitOutputsFunction(
-            new StarlarkImplicitOutputsFunctionWithCallback(callback, thread.getCallerLocation()));
+            new StarlarkImplicitOutputsFunctionWithCallback(callback));
       } else {
         builder.setImplicitOutputsFunction(
             new StarlarkImplicitOutputsFunctionWithMap(

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java

@@ -295,7 +295,7 @@ private void addOutput(String key, Object value)
           "Cannot add outputs to immutable Outputs object");
       if (outputs.containsKey(key)
           || (context.isExecutable() && EXECUTABLE_OUTPUT_NAME.equals(key))) {
-        throw new EvalException("Multiple outputs with the same key: " + key);
+        throw Starlark.errorf("Multiple outputs with the same key: %s", key);
       }
       outputs.put(key, value);
     }
@@ -365,11 +365,9 @@ public void repr(Printer printer) {
 
     private void checkMutable() throws EvalException {
       if (isImmutable()) {
-        throw new EvalException(
-            String.format(
-                "cannot access outputs of rule '%s' outside of its own "
-                    + "rule implementation function",
-                context.ruleLabelCanonicalName));
+        throw Starlark.errorf(
+            "cannot access outputs of rule '%s' outside of its own rule implementation function",
+            context.ruleLabelCanonicalName);
       }
     }
 
@@ -405,11 +403,10 @@ public void nullify() {
 
   public void checkMutable(String attrName) throws EvalException {
     if (isImmutable()) {
-      throw new EvalException(
-          String.format(
-              "cannot access field or method '%s' of rule context for '%s' outside of its own rule "
-                  + "implementation function",
-              attrName, ruleLabelCanonicalName));
+      throw Starlark.errorf(
+          "cannot access field or method '%s' of rule context for '%s' outside of its own rule "
+              + "implementation function",
+          attrName, ruleLabelCanonicalName);
     }
   }
 
@@ -593,10 +590,9 @@ public BuildConfiguration getHostConfiguration() throws EvalException {
   @Nullable
   public Object getBuildSettingValue() throws EvalException {
     if (ruleContext.getRule().getRuleClassObject().getBuildSetting() == null) {
-      throw new EvalException(
-          String.format(
-              "attempting to access 'build_setting_value' of non-build setting %s",
-              ruleLabelCanonicalName));
+      throw Starlark.errorf(
+          "attempting to access 'build_setting_value' of non-build setting %s",
+          ruleLabelCanonicalName);
     }
     ImmutableMap<Label, Object> starlarkFlagSettings =
         ruleContext.getConfiguration().getOptions().getStarlarkOptions();
@@ -719,7 +715,7 @@ public Sequence<String> tokenize(String optionString) throws EvalException {
     try {
       ShellUtils.tokenize(options, optionString);
     } catch (TokenizationException e) {
-      throw new EvalException(e.getMessage() + " while tokenizing '" + optionString + "'");
+      throw Starlark.errorf("%s while tokenizing '%s'", e.getMessage(), optionString);
     }
     return StarlarkList.immutableCopyOf(options);
   }
@@ -738,7 +734,7 @@ public String expand(
       }
       return LabelExpander.expand(expression, labelMap, labelResolver);
     } catch (NotUniqueExpansionException e) {
-      throw new EvalException(e.getMessage() + " while expanding '" + expression + "'");
+      throw Starlark.errorf("%s while expanding '%s'", e.getMessage(), expression);
     }
   }