[flake8-logging] Implement LOG007: ExceptionWithoutExcInfo#7410
Merged
charliermarsh merged 9 commits intoastral-sh:mainfrom Sep 18, 2023
Merged
[flake8-logging] Implement LOG007: ExceptionWithoutExcInfo#7410charliermarsh merged 9 commits intoastral-sh:mainfrom
flake8-logging] Implement LOG007: ExceptionWithoutExcInfo#7410charliermarsh merged 9 commits intoastral-sh:mainfrom
Conversation
flake8-logging] Implement LOG007: ExcInfoFalseInExceptionflake8-logging] Implement LOG007: ExcInfoFalseInException
Contributor
PR Check ResultsEcosystem✅ ecosystem check detected no changes. |
qdegraaf
commented
Sep 15, 2023
Comment on lines
+40
to
+43
| if checker | ||
| .semantic() | ||
| .resolve_call_path(call.func.as_ref()) | ||
| .is_some_and(|call_path| matches!(call_path.as_slice(), ["logging", "exception"])) |
Contributor
Author
There was a problem hiding this comment.
Would it be worth it to check the name of the call first before calling resolve_call_path()for performance reasons? Or would that just add unnecessary clutter?
konstin
reviewed
Sep 18, 2023
crates/ruff/src/rules/flake8_logging/rules/exc_info_false_in_exception.rs
Outdated
Show resolved
Hide resolved
|
|
||
| /// LOG007 | ||
| pub(crate) fn exc_info_false_in_exception(checker: &mut Checker, call: &ExprCall) { | ||
| if checker |
Member
There was a problem hiding this comment.
This needs to catch all log messages.
More common than
logging.exception("...")is doing
logger = logging.getLogger(__name__)
logger.exception("...")ruff has utils to detect logging calls, see is_logger_candidate and e.g. the G001 implementation.
Contributor
Author
There was a problem hiding this comment.
Rewrote to utilise is_logger_candidate. It flags logger.exception and logging.exception now but does not flag from logging import exception; exception() Is that a limitation of is_logger_candidate? And if so, do we want to create another branch of logic to catch those cases?
8f03a93 to
d42e6d6
Compare
flake8-logging] Implement LOG007: ExcInfoFalseInExceptionflake8-logging] Implement LOG007: ExceptionWithoutExcInfo
charliermarsh
approved these changes
Sep 18, 2023
renovate bot
referenced
this pull request
in allenporter/flux-local
Sep 24, 2023
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [ruff](https://docs.astral.sh/ruff) ([source](https://togithub.com/astral-sh/ruff), [changelog](https://togithub.com/astral-sh/ruff/releases)) | `==0.0.290` -> `==0.0.291` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>astral-sh/ruff (ruff)</summary> ### [`v0.0.291`](https://togithub.com/astral-sh/ruff/releases/tag/v0.0.291) [Compare Source](https://togithub.com/astral-sh/ruff/compare/v0.0.290...v0.0.291) <!-- Release notes generated using configuration in .github/release.yml at v0.0.291 --> #### What's Changed ##### Deprecations **The `format` command-line argument and configuration option has been renamed to `output-format`.** While Ruff will continue to respect `format` when passed as a command-line argument or configuration option, this backwards-compatible support will be dropped in a future release. See: [https://github.com/astral-sh/ruff/pull/7514](https://togithub.com/astral-sh/ruff/pull/7514). ##### Rules - \[`flake8-bandit`] Implement `S201`: `flask-debug-true` by [@​mkniewallner](https://togithub.com/mkniewallner) in [https://github.com/astral-sh/ruff/pull/7503](https://togithub.com/astral-sh/ruff/pull/7503) - \[`flake8-bandit`] Implement `S507`: `ssh_no_host_key_verification` by [@​mkniewallner](https://togithub.com/mkniewallner) in [https://github.com/astral-sh/ruff/pull/7528](https://togithub.com/astral-sh/ruff/pull/7528) - \[`flake8-logging`] Implement `LOG002`: `invalid-get-logger-argument` by [@​dhruvmanila](https://togithub.com/dhruvmanila) in [https://github.com/astral-sh/ruff/pull/7399](https://togithub.com/astral-sh/ruff/pull/7399) - \[`flake8-logging`] Implement `LOG007`: `exception-without-exc-info` by [@​qdegraaf](https://togithub.com/qdegraaf) in [https://github.com/astral-sh/ruff/pull/7410](https://togithub.com/astral-sh/ruff/pull/7410) - \[`refurb`] Implement `FURB140`: `reimplemented-starmap` by [@​SavchenkoValeriy](https://togithub.com/SavchenkoValeriy) in [https://github.com/astral-sh/ruff/pull/7253](https://togithub.com/astral-sh/ruff/pull/7253) - \[`refurb`] Implement `FURB148`: `unnecessary-enumerate` by [@​tjkuson](https://togithub.com/tjkuson) in [https://github.com/astral-sh/ruff/pull/7454](https://togithub.com/astral-sh/ruff/pull/7454) - \[`ruff`] Detect `asyncio.get_running_loop` calls in RUF006 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7562](https://togithub.com/astral-sh/ruff/pull/7562) ##### Settings - Show `--no-X` variants in CLI help by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7504](https://togithub.com/astral-sh/ruff/pull/7504) - Rename `format` option to `output-format` by [@​MichaReiser](https://togithub.com/MichaReiser) in [https://github.com/astral-sh/ruff/pull/7514](https://togithub.com/astral-sh/ruff/pull/7514) - Enable tab completion for `ruff rule` by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7560](https://togithub.com/astral-sh/ruff/pull/7560) ##### Bug Fixes - Add padding to prevent some autofix errors by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7461](https://togithub.com/astral-sh/ruff/pull/7461) - Remove parentheses when rewriting assert calls to statements by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7464](https://togithub.com/astral-sh/ruff/pull/7464) - Avoid flagging starred elements in C402 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7466](https://togithub.com/astral-sh/ruff/pull/7466) - Extend `bad-dunder-method-name` to permit `attrs` dunders by [@​tjkuson](https://togithub.com/tjkuson) in [https://github.com/astral-sh/ruff/pull/7472](https://togithub.com/astral-sh/ruff/pull/7472) - Avoid N802 violations for [@​overload](https://togithub.com/overload) methods by [@​JonathanPlasse](https://togithub.com/JonathanPlasse) in [https://github.com/astral-sh/ruff/pull/7498](https://togithub.com/astral-sh/ruff/pull/7498) - Avoid flagging starred expressions in UP007 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7505](https://togithub.com/astral-sh/ruff/pull/7505) - Ensure that LOG007 only triggers on `.exception()` calls by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7524](https://togithub.com/astral-sh/ruff/pull/7524) - Use strict sorted and union for NoQA mapping insertion by [@​dhruvmanila](https://togithub.com/dhruvmanila) in [https://github.com/astral-sh/ruff/pull/7531](https://togithub.com/astral-sh/ruff/pull/7531) - Avoid inserting imports directly after continuation by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7553](https://togithub.com/astral-sh/ruff/pull/7553) - Add padding in `PERF102` fixes by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7554](https://togithub.com/astral-sh/ruff/pull/7554) - Avoid invalid fix for parenthesized values in F601 by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7559](https://togithub.com/astral-sh/ruff/pull/7559) - Treat `os.error` as an `OSError` alias by [@​charliermarsh](https://togithub.com/charliermarsh) in [https://github.com/astral-sh/ruff/pull/7582](https://togithub.com/astral-sh/ruff/pull/7582) - Extend `bad-dunder-method-name` to permit `__html__` by [@​jaap3](https://togithub.com/jaap3) in [https://github.com/astral-sh/ruff/pull/7492](https://togithub.com/astral-sh/ruff/pull/7492) - Fix stylist indentation with a formfeed by [@​konstin](https://togithub.com/konstin) in [https://github.com/astral-sh/ruff/pull/7489](https://togithub.com/astral-sh/ruff/pull/7489) #### New Contributors - [@​MicaelJarniac](https://togithub.com/MicaelJarniac) made their first contribution in [https://github.com/astral-sh/ruff/pull/5498](https://togithub.com/astral-sh/ruff/pull/5498) - [@​maheshsaripalli9](https://togithub.com/maheshsaripalli9) made their first contribution in [https://github.com/astral-sh/ruff/pull/7552](https://togithub.com/astral-sh/ruff/pull/7552) - [@​T-256](https://togithub.com/T-256) made their first contribution in [https://github.com/astral-sh/ruff/pull/7585](https://togithub.com/astral-sh/ruff/pull/7585) **Full Changelog**: astral-sh/ruff@v0.0.290...v0.0.291 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/allenporter/flux-local). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi45Ny4xIiwidXBkYXRlZEluVmVyIjoiMzYuOTcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements a new rule for
flake8-loggingplugin that checks for uses oflogging.exception()withexc_infoset toFalseor a falsy value. It suggests usinglogging.errorin these cases instead.I am unsure about the name. Open to suggestions there, went with the most explicit name I could think of in the meantime.
Refer #7248
Test Plan
Added a new fixture cases and ran
cargo test