astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
30.94k stars 1.02k forks source link

Implement `flake8-logging` #7248

Open LaBatata101 opened 1 year ago

LaBatata101 commented 1 year ago

Issue to track the implementation of flake8-logging rules.

qdegraaf commented 1 year ago

I'm implementing LOG007 but will wait for review and merge of https://github.com/astral-sh/ruff/pull/7249 and the boilerplate is in so it's easier to review

LaBatata101 commented 1 year ago

I'm implementing LOG001, just waiting for #7249 to get merged

jonprindiville commented 1 year ago

Hm, LOG001 feels a bit like a specific case of banned-api (TID251).

Not to say that LOG001 can't have an explicit implementation, just reminds me of this other thing.

qdegraaf commented 12 months ago

It looks like LOG012 is covered by PLE1206|LoggingTooFewArgs it only appears to cover %s formatting but so does LOG012 according to upstream README

tjkuson commented 12 months ago

LOG007 should probably be implemented only once #4136 is resolved; otherwise, it conflicts with TRY400.

9128305 commented 9 months ago

New rules https://github.com/adamchainz/flake8-logging#log013-formatting-error-missingunreferenced-keys-keys https://github.com/adamchainz/flake8-logging#log014-avoid-exc_infotrue-outside-of-exception-handlers

dhruvmanila commented 9 months ago

Thanks! I've updated the issue description.

javulticat commented 8 months ago

Might I suggest that ruff drops/ignores flake8-logging-format (and the G error codes) entirely in favor of using flake8-logging (and exclusively the LOG error codes) as its source-of-truth for logging-related linting rules?

Currently, it is pretty confusing that ruff has logging-related linting rules spread out in two separate places (G and LOG). To an end user of ruff, there really shouldn't be two separate places to find logging-related linting rules - it really seems like it's just an implementation detail that they were originally derived from two separate projects.

To illustrate a concrete example of how this is a problem for our organization, I see here that you've marked that some of flake8-logging's LOG rules are already covered by some G rules from flake8-logging-format you already have, so you aren't planning on implementing those LOG rules (from what I gather, please correct me if I am wrong). As an organization that is currently using flake8 with flake8-logging, if we were to switch to ruff, how are our engineers supposed to know which of ruff's G rules map to the LOG rules they are familiar with (short of having our engineers digging through Github and finding this Issue, which isn't a realistic option)? I suppose you could put it in the official docs, but it would vastly reduce the friction for us to migrate to ruff if we could simply enable LOG rules and get the same behavior we currently get from flake8-logging. Because, even if the mapping from G to LOG rules was documented, is there any guarantee that the rules derived from flake8-logging-format have identical behavior as the corresponding rule(s) derived from flake8-logging?

That brings me to what is an even more important point, in my opinion. Compared to flake8-logging-format, flake8-logging seems to pretty objectively be a far better project to use as the source-of-truth from which you derive your logging-related linting rules. flake8-logging-format is "maintained" (and I use that word very lightly, because the project hasn't seen any activity in over a year) by some unknown corporate entity. On the other hand, flake8-logging was created and is actively maintained (most recent commit is less than a week old) by one of the most respected developers in the world of OSS Python software, adamchainz (whose Github profile should speak for itself, but highlights: core member of both Django and pytest teams, primary author/maintainer of many other highly regarded and heavily used OSS Python projects - including some I believe ruff is already using, such as flake8-comprehensions).

I encourage you to read Adam's blog post on why he created flake8-logging. Essentially, it sounds like he had his own concerns about using flake8-logging-format to enforce logging-related linting rules and decided to create a higher-quality, up-to-date flake8 plugin that contained a superset of those rules that would render flake8-logging-format obsolete: flake8-logging. So, if I understand that correctly, I believe you should be able to look at flake8-logging exclusively for (likely higher-quality) implementations of all of the rules that flake8-logging-format supports, plus all of the additional rules that flake8-logging has added (and will continue to add if/when helpful new rules are identified, such as the recent LOG013 and LOG014 rules, which is something flake8-logging-format hasn't done in years).

Thanks for considering! I've had a lot of my engineers ask about switching to ruff, and we've been watching the project with great interest, but this is definitely one of the larger blockers that are still preventing us from adopting it in my organization.

jakkdl commented 5 months ago

New rule LOG015 added upstream: https://github.com/adamchainz/flake8-logging?tab=readme-ov-file#log015-avoid-logging-calls-on-the-root-logger