jbudorick@, are you working on this now?

I'm working on porting our internal lint improvements from GYP to GN and then plan to upstream them. This issue will intersect with my changes so I can handle it as well - most likely, within next week.

I'm not working on this at the moment, and while I know agrieve has recently been doing some things with lint, I don't think this is one of them. (Correct me if I'm wrong on that.)

One of the two of us would be happy to review any upstreaming CLs.

Did some initial looking:

A library target's .build_config does track resource dirs as.
E.g.: gen/ui/android/ui_java.build_config
{
  "deps_info": {
    "deps_configs": [
      "gen/ui/android/ui_java_resources.build_config",
      "gen/base/base_java.build_config"
    ],
    "dex_path": "gen/ui/android/ui_java.dex.jar",
    "jar_path": "lib.java/ui/android/ui_java.jar",
    "name": "ui_java.build_config",
    "path": "gen/ui/android/ui_java.build_config",
    "requires_android": true,
    "resources_deps": [
      "gen/ui/android/ui_java_resources.build_config",
      "gen/ui/android/ui_strings_grd.build_config"
    ],
(snip)


Looking at gen/ui/android/ui_java_resources.build_config, we have:
{
  "deps_info": {
    "deps_configs": [
      "gen/ui/android/ui_strings_grd.build_config"
    ],
    "name": "ui_java_resources.build_config",
    "package_name": "org.chromium.ui",
    "path": "gen/ui/android/ui_java_resources.build_config",
    "r_text": "gen/ui/android/ui_java_resources_R.txt",
    "resources_zip": "gen/ui/android/ui_java_resources.resources.zip",
    "srcjar": "gen/ui/android/ui_java_resources.srcjar",
    "type": "android_resources"
  },
  "resources": {
    "dependency_zips": [
      "gen/ui/android/ui_strings_grd.resources.zip"
    ],
    "extra_package_names": [],
    "extra_r_text_files": []
  }
}


So, to fix this, I think we'd need to:
1. Update write_build_config.py to maintain a list of .zip files pointed to by "resources_deps"
2. Pass that list to lint.py
3. Have lint.py extract the files

If #3 is slow, we could maybe have it just extract text files and create 0-length files for .pngs

I'm writing this under the assumption that we are adding resource checking into an existing lint.py invocation in 'compile_java' template. We can in theory process resources separately but we'll not be able to use UnusedResources and MissingRegistered warnings. Another option is to run resources checks only when building APK but UnusedResources will not work too (AFAIK this check requires both resources and sources to be present to work properly).

We should avoid double-checking resources (e.g. checking content resources when processing content_java and then re-check again when processing chrome_java). One possible solution here is to check only resources from direct dependencies.

I also see some caveats with zip-based approach:
 - Original paths to files are lost so issues cannot be suppressed easily. However we already have this problem for Java files.
 - Resources in zips are processed - we'll get generated v14-compatible resources (which I think is okay) and we'll get crunched images (which may not be so good, especially for 9-patches).
 - If we try to use 0-length PNGs, then we have to give up lint's image checks.

We can pass a list of resource folders instead, falling back to zip files only if these resources are generated (e.g. by GRIT).

So, to summarize my proposal:
 1. Lint checks resources when building java library or APK (we do not add new places to invoke lint.py).
 2. Only direct "android_resources" dependencies are checked.
 3. Resources are passed to lint as paths to folders, unless there is no such folder - then resource zip is passed instead and unpacked by lint.py.
Basically, I just re-invented what GYP already does. 

What do you think?

Sounds perfect!

Just wanted to check in and see how this was going?

I have more or less working prototype now. There is a major roadblock, though: by design in GN we have android_resources targets completely separate from android_library ones (see https://codereview.chromium.org/361633002). R.java is generated in android_resources. In order to use R.java in android_library we have to directly depend on the corresponding resource target. So we have no way to determine whether resource dependency is target's "own" resources or resources from some other dependency. If we ignore this and just add all direct resource dependencies as resource folders then we get a lot of false positive UnusedResource warnings from library resources (because unused resources is source-level check and we don't have, for example, ui_java sources when linting chrome_java - so we get a lot of warnings if ui/android/java/res is added as a resource folder). We also get warnings from resources from support libraries. There are several ways out of this that I can think of:

1. lint only resources, assuming there is no corresponding Java code - works well with current project model but we're losing UnusedResource and MissingRegistered completely.
2. allow transitive propagation of R.java (i.e. srcjars) through dependencies, so instead of directly depending on ui_java_resources, chrome_java will depend on chrome_java_resources only. And it will get ui_java_resources through dependency on ui_java. This is what I did for prototype, but this required a few dependency changes here and there. It also doesn't prevent new direct resource dependencies from crawling in.
3. Separate "own" and dependency resource targets in the library target, e.g:
  android_library("chrome_java") {
    resource_deps = [ "chrome_java_resources" ]
    deps = [ "ui_java_resources", ... ]
    ...
  }
  This approach has no protection against accidentally misplaced dependencies.
4. Use (2) and disallow direct dependencies on android_resource targets outside of a single android_library target.

#2 would be my vote as well. Once this lands and we fix  bug #607761  then no new errors will be allowed to be made anyways.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18749bd362a7a8b7f4c1185c838d724dea5fad85

commit 18749bd362a7a8b7f4c1185c838d724dea5fad85
Author: mlopatkin <mlopatkin@yandex-team.ru>
Date: Mon Jun 20 18:13:43 2016

[GN] Lint Android resources

Lint is still run only within android_library/android_apk targets but resources
folders and generated resources ZIPs from direct dependencies are added to this
run. ZIPs are extracted to temporary folders during linting.

This approach requires a big rewrite of existing dependencies. Before the patch
every library that needs to reference something from R.java of the other library
has to directly depend on the resource target that generates this R.java, for
example chrome_java depended on ui_java_resources while also dependening on
ui_java. This is the only way to get R.java from ui_java_resources in
chrome_java. This causes Lint to lint ui_java_resources twice (within
chrome_java and ui_java) and may produce false positives. This patch changes
these R.java dependencies to transitives, so chrome_java only depends on ui_java
and still gets all R.java from its resources.

Only resources that are the part of the library/APK should be direct
dependencies of this library otherwise Lint will not work properly.

Resource folders/ZIPs are propagated to dependent libraries via build_config
files. Each android_resources target publishes its resource folders in its
build_config; it also writes list of resources_zips of direct dependencies which
have no own package names - these are generated resources, conceptually part of
the same target. Library/APK targets expose list of folders and ZIPs from
dependencies via their own build_config which is read by Lint.

BUG= 595810 

Review-Url: https://codereview.chromium.org/2062633002
Cr-Commit-Position: refs/heads/master@{#400727}

[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/android_webview/glue/glue.gni
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/blimp/client/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/build/android/gyp/lint.py
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/build/android/gyp/write_build_config.py
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/build/config/android/internal_rules.gni
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/build/config/android/rules.gni
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/chrome/android/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/chrome/test/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/chrome/test/android/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/components/web_contents_delegate_android/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/content/public/android/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/content/shell/android/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/remoting/android/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/third_party/android_media/BUILD.gn
[modify] https://crrev.com/18749bd362a7a8b7f4c1185c838d724dea5fad85/ui/android/BUILD.gn

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/081933c77ddbcf8907791a683e43edfd81b4f926

commit 081933c77ddbcf8907791a683e43edfd81b4f926
Author: mlopatkin <mlopatkin@yandex-team.ru>
Date: Mon Jun 20 18:13:43 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/baa5f9619c223582f92599c28a22ad2c1800c39b

commit baa5f9619c223582f92599c28a22ad2c1800c39b
Author: mlopatkin <mlopatkin@yandex-team.ru>
Date: Mon Jun 20 18:13:43 2016