Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a01a9374 authored by Adam Lesinski's avatar Adam Lesinski
Browse files

Reduce warning verbosity in aapt

- Attributed source of problems to the correct file.
- Only verify string localizations against valid
  locales.
Bug:13140015
Change-Id: I9dabc5efa0510649caee8af0c8ebb803d6f48269
parent 40436a20
Loading
Loading
Loading
Loading
+45 −28
Original line number Diff line number Diff line
@@ -1317,9 +1317,9 @@ status_t compileResourceFile(Bundle* bundle,
                        curIsFormatted = false;
                        // Untranslatable strings must only exist in the default [empty] locale
                        if (locale.size() > 0) {
                            fprintf(stderr, "aapt: warning: string '%s' in %s marked untranslatable but exists"
                                    " in locale '%s'\n", String8(name).string(),
                                    bundle->getResourceSourceDirs()[0],
                            SourcePos(in->getPrintableSource(), block.getLineNumber()).warning(
                                    "string '%s' marked untranslatable but exists in locale '%s'\n",
                                    String8(name).string(),
                                    locale.string());
                            // hasErrors = localHasErrors = true;
                        } else {
@@ -1330,7 +1330,10 @@ status_t compileResourceFile(Bundle* bundle,
                            // having no default translation.
                        }
                    } else {
                        outTable->addLocalization(name, locale);
                        outTable->addLocalization(
                                name,
                                locale,
                                SourcePos(in->getPrintableSource(), block.getLineNumber()));
                    }

                    if (formatted == false16) {
@@ -2570,9 +2573,9 @@ status_t ResourceTable::addSymbols(const sp<AaptSymbols>& outSymbols) {


void
ResourceTable::addLocalization(const String16& name, const String8& locale)
ResourceTable::addLocalization(const String16& name, const String8& locale, const SourcePos& src)
{
    mLocalizations[name].insert(locale);
    mLocalizations[name][locale] = src;
}


@@ -2592,21 +2595,22 @@ ResourceTable::validateLocalizations(void)
    const String8 defaultLocale;

    // For all strings...
    for (map<String16, set<String8> >::iterator nameIter = mLocalizations.begin();
    for (map<String16, map<String8, SourcePos> >::iterator nameIter = mLocalizations.begin();
         nameIter != mLocalizations.end();
         nameIter++) {
        const set<String8>& configSet = nameIter->second;   // naming convenience
        const map<String8, SourcePos>& configSrcMap = nameIter->second;

        // Look for strings with no default localization
        if (configSet.count(defaultLocale) == 0) {
            fprintf(stdout, "aapt: warning: string '%s' has no default translation in %s; found:",
                    String8(nameIter->first).string(), mBundle->getResourceSourceDirs()[0]);
            for (set<String8>::const_iterator locales = configSet.begin();
                 locales != configSet.end();
        if (configSrcMap.count(defaultLocale) == 0) {
            SourcePos().warning("string '%s' has no default translation.",
                    String8(nameIter->first).string());
            if (mBundle->getVerbose()) {
                for (map<String8, SourcePos>::const_iterator locales = configSrcMap.begin();
                    locales != configSrcMap.end();
                    locales++) {
                fprintf(stdout, " %s", (*locales).string());
                    locales->second.printf("locale %s found", locales->first.string());
                }
            }
            fprintf(stdout, "\n");
            // !!! TODO: throw an error here in some circumstances
        }

@@ -2616,6 +2620,8 @@ ResourceTable::validateLocalizations(void)
            const char* start = allConfigs;
            const char* comma;
            
            set<String8> missingConfigs;
            AaptLocaleValue locale;
            do {
                String8 config;
                comma = strchr(start, ',');
@@ -2626,27 +2632,38 @@ ResourceTable::validateLocalizations(void)
                    config.setTo(start);
                }

                if (!locale.initFromFilterString(config)) {
                    continue;
                }

                // don't bother with the pseudolocale "zz_ZZ"
                if (config != "zz_ZZ") {
                    if (configSet.find(config) == configSet.end()) {
                    if (configSrcMap.find(config) == configSrcMap.end()) {
                        // okay, no specific localization found.  it's possible that we are
                        // requiring a specific regional localization [e.g. de_DE] but there is an
                        // available string in the generic language localization [e.g. de];
                        // consider that string to have fulfilled the localization requirement.
                        String8 region(config.string(), 2);
                        if (configSet.find(region) == configSet.end()) {
                            if (configSet.count(defaultLocale) == 0) {
                                fprintf(stdout, "aapt: warning: "
                                        "**** string '%s' has no default or required localization "
                                        "for '%s' in %s\n",
                                        String8(nameIter->first).string(),
                                        config.string(),
                                        mBundle->getResourceSourceDirs()[0]);
                            }
                        if (configSrcMap.find(region) == configSrcMap.end() &&
                                configSrcMap.count(defaultLocale) == 0) {
                            missingConfigs.insert(config);
                        }
                    }
                }
            } while (comma != NULL);

            if (!missingConfigs.empty()) {
                String8 configStr;
                for (set<String8>::iterator iter = missingConfigs.begin();
                     iter != missingConfigs.end();
                     iter++) {
                    configStr.appendFormat(" %s", iter->string());
                }
                SourcePos().warning("string '%s' is missing %u required localizations:%s",
                        String8(nameIter->first).string(),
                        (unsigned int)missingConfigs.size(),
                        configStr.string());
            }
        }
    }

+2 −2
Original line number Diff line number Diff line
@@ -220,7 +220,7 @@ public:

    status_t assignResourceIds();
    status_t addSymbols(const sp<AaptSymbols>& outSymbols = NULL);
    void addLocalization(const String16& name, const String8& locale);
    void addLocalization(const String16& name, const String8& locale, const SourcePos& src);
    status_t validateLocalizations(void);

    status_t flatten(Bundle*, const sp<AaptFile>& dest);
@@ -551,7 +551,7 @@ private:
    Bundle* mBundle;
    
    // key = string resource name, value = set of locales in which that name is defined
    map<String16, set<String8> > mLocalizations;
    map<String16, map<String8, SourcePos> > mLocalizations;
};

#endif
+48 −60
Original line number Diff line number Diff line
@@ -10,17 +10,20 @@ using namespace std;
// =============================================================================
struct ErrorPos
{
    enum Level {
        NOTE,
        WARNING,
        ERROR
    };

    String8 file;
    int line;
    String8 error;
    bool fatal;
    Level level;

    ErrorPos();
    ErrorPos(const ErrorPos& that);
    ErrorPos(const String8& file, int line, const String8& error, bool fatal);
    ~ErrorPos();
    bool operator<(const ErrorPos& rhs) const;
    bool operator==(const ErrorPos& rhs) const;
    ErrorPos(const String8& file, int line, const String8& error, Level level);
    ErrorPos& operator=(const ErrorPos& rhs);

    void print(FILE* to) const;
@@ -29,7 +32,7 @@ struct ErrorPos
static vector<ErrorPos> g_errors;

ErrorPos::ErrorPos()
    :line(-1), fatal(false)
    :line(-1), level(NOTE)
{
}

@@ -37,41 +40,16 @@ ErrorPos::ErrorPos(const ErrorPos& that)
    :file(that.file),
     line(that.line),
     error(that.error),
     fatal(that.fatal)
     level(that.level)
{
}

ErrorPos::ErrorPos(const String8& f, int l, const String8& e, bool fat)
ErrorPos::ErrorPos(const String8& f, int l, const String8& e, Level lev)
    :file(f),
     line(l),
     error(e),
     fatal(fat)
{
}

ErrorPos::~ErrorPos()
{
}

bool
ErrorPos::operator<(const ErrorPos& rhs) const
     level(lev)
{
    if (this->file < rhs.file) return true;
    if (this->file == rhs.file) {
        if (this->line < rhs.line) return true;
        if (this->line == rhs.line) {
            if (this->error < rhs.error) return true;
        }
    }
    return false;
}

bool
ErrorPos::operator==(const ErrorPos& rhs) const
{
    return this->file == rhs.file
            && this->line == rhs.line
            && this->error == rhs.error;
}

ErrorPos&
@@ -80,19 +58,35 @@ ErrorPos::operator=(const ErrorPos& rhs)
    this->file = rhs.file;
    this->line = rhs.line;
    this->error = rhs.error;
    this->level = rhs.level;
    return *this;
}

void
ErrorPos::print(FILE* to) const
{
    const char* type = fatal ? "error:" : "warning:";
    const char* type = "";
    switch (level) {
    case NOTE:
        type = "note: ";
        break;
    case WARNING:
        type = "warning: ";
        break;
    case ERROR:
        type = "error: ";
        break;
    }
    
    if (!this->file.isEmpty()) {
        if (this->line >= 0) {
            fprintf(to, "%s:%d: %s%s\n", this->file.string(), this->line, type, this->error.string());
        } else {
            fprintf(to, "%s: %s%s\n", this->file.string(), type, this->error.string());
        }
    } else {
        fprintf(to, "%s%s\n", type, this->error.string());
    }
}

// SourcePos
@@ -116,40 +110,34 @@ SourcePos::~SourcePos()
{
}

int
void
SourcePos::error(const char* fmt, ...) const
{
    int retval=0;
    char buf[1024];
    va_list ap;
    va_start(ap, fmt);
    retval = vsnprintf(buf, sizeof(buf), fmt, ap);
    String8 msg = String8::formatV(fmt, ap);
    va_end(ap);
    char* p = buf + retval - 1;
    while (p > buf && *p == '\n') {
        *p = '\0';
        p--;
    }
    g_errors.push_back(ErrorPos(this->file, this->line, String8(buf), true));
    return retval;
    g_errors.push_back(ErrorPos(this->file, this->line, msg, ErrorPos::ERROR));
}

int
void
SourcePos::warning(const char* fmt, ...) const
{
    int retval=0;
    char buf[1024];
    va_list ap;
    va_start(ap, fmt);
    retval = vsnprintf(buf, sizeof(buf), fmt, ap);
    String8 msg = String8::formatV(fmt, ap);
    va_end(ap);
    char* p = buf + retval - 1;
    while (p > buf && *p == '\n') {
        *p = '\0';
        p--;
    ErrorPos(this->file, this->line, msg, ErrorPos::WARNING).print(stderr);
}
    ErrorPos(this->file, this->line, String8(buf), false).print(stderr);
    return retval;

void
SourcePos::printf(const char* fmt, ...) const
{
    va_list ap;
    va_start(ap, fmt);
    String8 msg = String8::formatV(fmt, ap);
    va_end(ap);
    ErrorPos(this->file, this->line, msg, ErrorPos::NOTE).print(stderr);
}

bool
+3 −2
Original line number Diff line number Diff line
@@ -17,8 +17,9 @@ public:
    SourcePos();
    ~SourcePos();

    int error(const char* fmt, ...) const;
    int warning(const char* fmt, ...) const;
    void error(const char* fmt, ...) const;
    void warning(const char* fmt, ...) const;
    void printf(const char* fmt, ...) const;

    static bool hasErrors();
    static void printErrors(FILE* to);