AAPT2: Warn when positional arguments exist and --legacy is on
This is normally an error, but old AAPT didn't check for it correctly,
so many projects violate this. With --legacy, this becomes a warning.
Change-Id: I23647e029930e11b719591cd38609e1b43247e20
diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp
index e1f9642..5e7d3ec 100644
--- a/tools/aapt2/ResourceParser.cpp
+++ b/tools/aapt2/ResourceParser.cpp
@@ -582,10 +582,15 @@
if (formatted && translateable) {
if (!util::verifyJavaStringFormat(*stringValue->value)) {
- mDiag->error(DiagMessage(outResource->source)
- << "multiple substitutions specified in non-positional format; "
- "did you mean to add the formatted=\"false\" attribute?");
- return false;
+ DiagMessage msg(outResource->source);
+ msg << "multiple substitutions specified in non-positional format; "
+ "did you mean to add the formatted=\"false\" attribute?";
+ if (mOptions.errorOnPositionalArguments) {
+ mDiag->error(msg);
+ return false;
+ }
+
+ mDiag->warn(msg);
}
}
diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h
index 9ad749e..51cbbe1 100644
--- a/tools/aapt2/ResourceParser.h
+++ b/tools/aapt2/ResourceParser.h
@@ -44,6 +44,11 @@
* Whether the default setting for this parser is to allow translation.
*/
bool translatable = true;
+
+ /**
+ * Whether positional arguments in formatted strings are treated as errors or warnings.
+ */
+ bool errorOnPositionalArguments = true;
};
/*
diff --git a/tools/aapt2/compile/Compile.cpp b/tools/aapt2/compile/Compile.cpp
index b3b0f65..c78670f 100644
--- a/tools/aapt2/compile/Compile.cpp
+++ b/tools/aapt2/compile/Compile.cpp
@@ -107,6 +107,7 @@
Maybe<std::string> resDir;
std::vector<std::u16string> products;
bool pseudolocalize = false;
+ bool legacyMode = false;
bool verbose = false;
};
@@ -192,6 +193,7 @@
ResourceParserOptions parserOptions;
parserOptions.products = options.products;
+ parserOptions.errorOnPositionalArguments = !options.legacyMode;
// If the filename includes donottranslate, then the default translatable is false.
parserOptions.translatable = pathData.name.find(u"donottranslate") == std::string::npos;
@@ -438,6 +440,8 @@
.optionalFlag("--dir", "Directory to scan for resources", &options.resDir)
.optionalSwitch("--pseudo-localize", "Generate resources for pseudo-locales "
"(en-XA and ar-XB)", &options.pseudolocalize)
+ .optionalSwitch("--legacy", "Treat errors that used to be valid in AAPT as warnings",
+ &options.legacyMode)
.optionalSwitch("-v", "Enables verbose logging", &options.verbose);
if (!flags.parse("aapt2 compile", args, &std::cerr)) {
return 1;
diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp
index 652e52f..8a87d96 100644
--- a/tools/aapt2/link/Link.cpp
+++ b/tools/aapt2/link/Link.cpp
@@ -228,29 +228,53 @@
return {};
}
+ /**
+ * Precondition: ResourceTable doesn't have any IDs assigned yet, nor is it linked.
+ * Postcondition: ResourceTable has only one package left. All others are stripped, or there
+ * is an error and false is returned.
+ */
bool verifyNoExternalPackages() {
+ auto isExtPackageFunc = [&](const std::unique_ptr<ResourceTablePackage>& pkg) -> bool {
+ return mContext.getCompilationPackage() != pkg->name ||
+ !pkg->id ||
+ pkg->id.value() != mContext.getPackageId();
+ };
+
bool error = false;
for (const auto& package : mFinalTable.packages) {
- if (mContext.getCompilationPackage() != package->name ||
- !package->id || package->id.value() != mContext.getPackageId()) {
+ if (isExtPackageFunc(package)) {
// We have a package that is not related to the one we're building!
for (const auto& type : package->types) {
for (const auto& entry : type->entries) {
+ ResourceNameRef resName(package->name, type->type, entry->name);
+
for (const auto& configValue : entry->values) {
- mContext.getDiagnostics()->error(
- DiagMessage(configValue.value->getSource())
- << "defined resource '"
- << ResourceNameRef(package->name,
- type->type,
- entry->name)
- << "' for external package '"
- << package->name << "'");
- error = true;
+ // Special case the occurrence of an ID that is being generated for the
+ // 'android' package. This is due to legacy reasons.
+ if (valueCast<Id>(configValue.value.get()) &&
+ package->name == u"android") {
+ mContext.getDiagnostics()->warn(
+ DiagMessage(configValue.value->getSource())
+ << "generated id '" << resName
+ << "' for external package '" << package->name
+ << "'");
+ } else {
+ mContext.getDiagnostics()->error(
+ DiagMessage(configValue.value->getSource())
+ << "defined resource '" << resName
+ << "' for external package '" << package->name
+ << "'");
+ error = true;
+ }
}
}
}
}
}
+
+ auto newEndIter = std::remove_if(mFinalTable.packages.begin(), mFinalTable.packages.end(),
+ isExtPackageFunc);
+ mFinalTable.packages.erase(newEndIter, mFinalTable.packages.end());
return !error;
}