You're right. The current version of the Pylint analyzer doesn't look at any other files in the directory, doesn't check imported modules, so that check doesn't work in the case of star imports.

I think that while this is the case we'll just have to disable that check.

(CL link: https://chromium-review.googlesource.com/c/chromium/src/+/1258087/2)

That's a bummer. In a previous CL it did find a typo in a method name in an infrequently used code path. (Python!)

Yeah :-P Anyway, this is an important thing to note, since this provides one more example of something in Pylint that works once you can recursively import and check all dependencies.

In the near term, a presubmit check might be more useful than tricium for checking this.

Would it be possible just to import explicitly instead of doing a wildcard import? PEP-8 discourages wildcards anyway, and if we imported IntegerTypes explicitly (which IIUC is a quick string substitution) we can retain the undefined variable check and its usefulness.

I wonder if we could annotate this error explicitly to say "This error can be caused by  'from foo import *' statements. These are are against the style guide, please fix the code"

This is bad code practice, and the warnings are fully worth the pain incurred by poor code hygiene.

Another option is to throttle the number of comments per category shown on gerrit to maximum of 2 per CL, server side. Quinten, wdyt?

Adding an extra note to "undefined-variable" warnings sounds good to me.

Throttling the number of comments per category to 2 seems potentially confusing to me; it might seem like Tricium found some occurrences but somehow failed to find other occurrences in other files.

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/81f0ccd71a28288fd5f1f82b0384b24f3b5ad5db

commit 81f0ccd71a28288fd5f1f82b0384b24f3b5ad5db
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Thu Nov 15 18:18:29 2018

[tricium pylint] Add a special-case message for undefined-variable

Bug:  891748 
Change-Id: Ia0d6a8046b13d096b9a79bcb9e02193dcf899f0e
Reviewed-on: https://chromium-review.googlesource.com/c/1332793
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Jao-ke Chin-Lee <jchinlee@chromium.org>
Reviewed-by: Marc-Antoine Ruel <maruel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#19014}
[modify] https://crrev.com/81f0ccd71a28288fd5f1f82b0384b24f3b5ad5db/go/src/infra/tricium/functions/pylint/pylint_parser.go
[modify] https://crrev.com/81f0ccd71a28288fd5f1f82b0384b24f3b5ad5db/go/src/infra/tricium/functions/pylint/pylint_parser_test.go

New version of pylint deployed; the resolution we decided on for now is to keep the warning but add a note about avoiding wildcard imports when possible.