From 889bc0b523b47eeb38a72bf9bb6858ee525a7c7e Mon Sep 17 00:00:00 2001 From: adonovan Date: Fri, 7 Aug 2020 18:35:24 -0700 Subject: [PATCH] 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 --- .../StarlarkProviderValidationUtil.java | 13 ++-- .../lib/analysis/actions/SpawnAction.java | 5 +- .../build/lib/analysis/starlark/Args.java | 10 ++- .../starlark/FunctionTransitionUtil.java | 44 +++++-------- .../starlark/StarlarkActionFactory.java | 21 +++---- .../analysis/starlark/StarlarkAttrModule.java | 22 +++---- .../starlark/StarlarkErrorReporter.java | 3 +- .../starlark/StarlarkRuleClassFunctions.java | 2 +- .../starlark/StarlarkRuleContext.java | 30 ++++----- .../bazel/repository/DecompressorValue.java | 11 ++-- .../starlark/StarlarkRepositoryContext.java | 36 +++++------ .../lib/packages/AttributeValueSource.java | 10 +-- .../lib/packages/BazelStarlarkContext.java | 7 ++- .../lib/packages/ConstantRuleVisibility.java | 3 +- .../lib/packages/DefaultPackageArguments.java | 8 +-- .../lib/packages/ImplicitOutputsFunction.java | 30 +++------ .../build/lib/packages/PackageFactory.java | 2 +- .../lib/packages/StarlarkNativeModule.java | 2 +- .../devtools/build/lib/packages/Type.java | 4 +- .../build/lib/packages/WorkspaceFactory.java | 8 +-- .../build/lib/rules/cpp/CcCommon.java | 20 +++--- .../lib/rules/cpp/CcToolchainFeatures.java | 62 ++++++++----------- .../lib/rules/cpp/CcToolchainProvider.java | 11 ++-- .../rules/cpp/CcToolchainProviderHelper.java | 3 +- .../lib/rules/cpp/CcToolchainVariables.java | 12 ++-- .../lib/rules/cpp/CompileBuildVariables.java | 5 +- .../build/lib/rules/cpp/CppLinkAction.java | 5 +- .../lib/rules/java/JavaCompileAction.java | 5 +- .../lib/rules/objc/AppleStarlarkCommon.java | 2 +- .../build/lib/rules/objc/ObjcProvider.java | 28 ++++----- .../build/lib/rules/python/PyStructUtils.java | 2 +- .../rules/repository/RepositoryFunction.java | 6 +- .../repository/WorkspaceAttributeMapper.java | 15 +++-- .../skyframe/StarlarkBuiltinsFunction.java | 11 +--- .../build/lib/syntax/StringModule.java | 2 +- 35 files changed, 196 insertions(+), 264 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/StarlarkProviderValidationUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/StarlarkProviderValidationUtil.java index 046a3c94740224..e18cd98c8dccce 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/StarlarkProviderValidationUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/StarlarkProviderValidationUtil.java @@ -18,6 +18,7 @@ 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 { @@ -25,17 +26,17 @@ public static void validateArtifacts(RuleContext ruleContext) throws EvalExcepti ImmutableSet 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 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)); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index d369363f59091e..b20f8e403e32ee 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/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 getStarlarkArgs() throws EvalException { public Sequence getStarlarkArgv() throws EvalException { try { return StarlarkList.immutableCopyOf(getArguments()); - } catch (CommandLineExpansionException exception) { - throw new EvalException(Location.BUILTIN, exception); + } catch (CommandLineExpansionException ex) { + throw new EvalException(ex); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java index 2aa929af735c09..3a8c165e29f6dd 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java +++ b/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; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java index 6c6d2fb93a8f9a..3e795c405761ae 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java @@ -102,8 +102,7 @@ static Map 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 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()); } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java index 9264d1e8dbc02e..0297222c8e1bff 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java @@ -452,9 +452,9 @@ public void runShell( List 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 { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java index 0a22b5338fd07b..4adf858170a0da 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java +++ b/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 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> 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)); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkErrorReporter.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkErrorReporter.java index 80d70a3ca4b332..96b5fcad0efb69 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkErrorReporter.java +++ b/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()); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index 141599853ec517..00101b8403decb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/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( diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java index cca3bfc4f4a8a2..cd59772c5e3ad2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java +++ b/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 starlarkFlagSettings = ruleContext.getConfiguration().getOptions().getStarlarkOptions(); @@ -719,7 +715,7 @@ public Sequence 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); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorValue.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorValue.java index b1a39d43cf262c..f0f71e3eeba0e2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/DecompressorValue.java @@ -16,7 +16,7 @@ import com.google.common.base.Optional; import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; -import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.Starlark; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; @@ -110,11 +110,10 @@ static Decompressor getDecompressor(Path archivePath) return TarBz2Function.INSTANCE; } else { throw new RepositoryFunctionException( - new EvalException( - String.format( - "Expected a file with a .zip, .jar, .war, .tar, .tar.gz, .tgz, .tar.xz, .txz, or " - + ".tar.bz2 suffix (got %s)", - archivePath)), + Starlark.errorf( + "Expected a file with a .zip, .jar, .war, .tar, .tar.gz, .tgz, .tar.xz, .txz, or " + + ".tar.bz2 suffix (got %s)", + archivePath), Transience.PERSISTENT); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java index 02fd4478995271..5db671e35c02f1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java @@ -479,12 +479,11 @@ private void validateExecuteArguments(Sequence arguments) throws EvalExceptio Object arg = arguments.get(i); if (isRemotable) { if (!(arg instanceof String || arg instanceof Label)) { - throw new EvalException("Argument " + i + " of execute is neither a label nor a string."); + throw Starlark.errorf("Argument %d of execute is neither a label nor a string.", i); } } else { if (!(arg instanceof String || arg instanceof Label || arg instanceof StarlarkPath)) { - throw new EvalException( - "Argument " + i + " of execute is neither a path, label, nor string."); + throw Starlark.errorf("Argument %d of execute is neither a path, label, nor string.", i); } } } @@ -992,11 +991,10 @@ private static ImmutableList checkAllUrls(Iterable urlList) throws Ev for (Object o : urlList) { if (!(o instanceof String)) { - throw new EvalException( - String.format( - "Expected a string or sequence of strings for 'url' argument, " - + "but got '%s' item in the sequence", - Starlark.type(o))); + throw Starlark.errorf( + "Expected a string or sequence of strings for 'url' argument, but got '%s' item in the" + + " sequence", + Starlark.type(o)); } result.add((String) o); } @@ -1148,10 +1146,10 @@ private static Map> getAuthHeaders(Map> getAuthHeaders(Map> getAuthHeaders(Map labels) throws EvalExc for (Label label : labels) { visibility = tryParse(label); if (visibility != null) { - throw new EvalException( + throw Starlark.errorf( "Public or private visibility labels (e.g. //visibility:public or" + " //visibility:private) cannot be used in combination with other labels"); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/DefaultPackageArguments.java b/src/main/java/com/google/devtools/build/lib/packages/DefaultPackageArguments.java index e8c1b6d51cb29a..ffca6d7ef88077 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/DefaultPackageArguments.java +++ b/src/main/java/com/google/devtools/build/lib/packages/DefaultPackageArguments.java @@ -49,12 +49,8 @@ private DefaultVisibility() { @Override protected void process(Package.Builder pkgBuilder, Location location, List