Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-logging] Implement LOG007: ExceptionWithoutExcInfo #7410

Merged
merged 9 commits into from Sep 18, 2023

Conversation

qdegraaf
Copy link
Contributor

Summary

This PR implements a new rule for flake8-logging plugin that checks for uses of logging.exception() with exc_info set to False or a falsy value. It suggests using logging.error in 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

@qdegraaf qdegraaf changed the title [flake8-logging] Implement LOG007: ExcInfoFalseInException [flake8-logging] Implement LOG007: ExcInfoFalseInException Sep 15, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 15, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

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"]))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?


/// LOG007
pub(crate) fn exc_info_false_in_exception(checker: &mut Checker, call: &ExprCall) {
if checker
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved this to #7502

@charliermarsh charliermarsh changed the title [flake8-logging] Implement LOG007: ExcInfoFalseInException [flake8-logging] Implement LOG007: ExceptionWithoutExcInfo Sep 18, 2023
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Sep 18, 2023
@charliermarsh charliermarsh enabled auto-merge (squash) September 18, 2023 17:29
@charliermarsh charliermarsh merged commit bccba5d into astral-sh:main Sep 18, 2023
15 checks passed
@qdegraaf qdegraaf deleted the rule/LOG007 branch September 20, 2023 09:55
renovate bot added a commit to allenporter/flux-local that referenced this pull request Sep 24, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](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` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/ruff/0.0.291?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/ruff/0.0.291?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/ruff/0.0.290/0.0.291?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/ruff/0.0.290/0.0.291?slim=true)](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:
[astral-sh/ruff#7514.

##### Rules

- \[`flake8-bandit`] Implement `S201`: `flask-debug-true` by
[@&#8203;mkniewallner](https://togithub.com/mkniewallner) in
[astral-sh/ruff#7503
- \[`flake8-bandit`] Implement `S507`: `ssh_no_host_key_verification` by
[@&#8203;mkniewallner](https://togithub.com/mkniewallner) in
[astral-sh/ruff#7528
- \[`flake8-logging`] Implement `LOG002`: `invalid-get-logger-argument`
by [@&#8203;dhruvmanila](https://togithub.com/dhruvmanila) in
[astral-sh/ruff#7399
- \[`flake8-logging`] Implement `LOG007`: `exception-without-exc-info`
by [@&#8203;qdegraaf](https://togithub.com/qdegraaf) in
[astral-sh/ruff#7410
- \[`refurb`] Implement `FURB140`: `reimplemented-starmap` by
[@&#8203;SavchenkoValeriy](https://togithub.com/SavchenkoValeriy) in
[astral-sh/ruff#7253
- \[`refurb`] Implement `FURB148`: `unnecessary-enumerate` by
[@&#8203;tjkuson](https://togithub.com/tjkuson) in
[astral-sh/ruff#7454
- \[`ruff`] Detect `asyncio.get_running_loop` calls in RUF006 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7562

##### Settings

- Show `--no-X` variants in CLI help by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7504
- Rename `format` option to `output-format` by
[@&#8203;MichaReiser](https://togithub.com/MichaReiser) in
[astral-sh/ruff#7514
- Enable tab completion for `ruff rule` by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7560

##### Bug Fixes

- Add padding to prevent some autofix errors by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7461
- Remove parentheses when rewriting assert calls to statements by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7464
- Avoid flagging starred elements in C402 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7466
- Extend `bad-dunder-method-name` to permit `attrs` dunders by
[@&#8203;tjkuson](https://togithub.com/tjkuson) in
[astral-sh/ruff#7472
- Avoid N802 violations for
[@&#8203;overload](https://togithub.com/overload) methods by
[@&#8203;JonathanPlasse](https://togithub.com/JonathanPlasse) in
[astral-sh/ruff#7498
- Avoid flagging starred expressions in UP007 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7505
- Ensure that LOG007 only triggers on `.exception()` calls by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7524
- Use strict sorted and union for NoQA mapping insertion by
[@&#8203;dhruvmanila](https://togithub.com/dhruvmanila) in
[astral-sh/ruff#7531
- Avoid inserting imports directly after continuation by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7553
- Add padding in `PERF102` fixes by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7554
- Avoid invalid fix for parenthesized values in F601 by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7559
- Treat `os.error` as an `OSError` alias by
[@&#8203;charliermarsh](https://togithub.com/charliermarsh) in
[astral-sh/ruff#7582
- Extend `bad-dunder-method-name` to permit `__html__` by
[@&#8203;jaap3](https://togithub.com/jaap3) in
[astral-sh/ruff#7492
- Fix stylist indentation with a formfeed by
[@&#8203;konstin](https://togithub.com/konstin) in
[astral-sh/ruff#7489

#### New Contributors

- [@&#8203;MicaelJarniac](https://togithub.com/MicaelJarniac) made their
first contribution in
[astral-sh/ruff#5498
- [@&#8203;maheshsaripalli9](https://togithub.com/maheshsaripalli9) made
their first contribution in
[astral-sh/ruff#7552
- [@&#8203;T-256](https://togithub.com/T-256) made their first
contribution in
[astral-sh/ruff#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants