From e88275280b19569d421d4a59c562e28afdb1e417 Mon Sep 17 00:00:00 2001 From: "Edgar R. M" Date: Thu, 26 Jan 2023 11:58:10 -0600 Subject: [PATCH] Implement some rules from `flake8-logging-format` (#2150) --- LICENSE | 205 +++++++++++++++ README.md | 22 ++ .../fixtures/flake8_logging_format/G001.py | 3 + .../fixtures/flake8_logging_format/G002.py | 3 + .../fixtures/flake8_logging_format/G003.py | 3 + .../fixtures/flake8_logging_format/G004.py | 4 + .../fixtures/flake8_logging_format/G010.py | 3 + .../fixtures/flake8_logging_format/G101_1.py | 8 + .../fixtures/flake8_logging_format/G101_2.py | 8 + .../fixtures/flake8_logging_format/G201.py | 3 + .../fixtures/flake8_logging_format/G202.py | 3 + .../G_argparse_parser_error_ok.py | 7 + .../flake8_logging_format/G_extra_ok.py | 8 + .../G_extra_str_format_ok.py | 8 + .../flake8_logging_format/G_simple_ok.py | 3 + .../flake8_logging_format/G_warnings_ok.py | 3 + ruff.schema.json | 16 ++ src/ast/helpers.rs | 15 ++ src/checkers/ast.rs | 22 +- src/registry.rs | 12 + src/rules/flake8_logging_format/mod.rs | 50 ++++ src/rules/flake8_logging_format/rules.rs | 235 ++++++++++++++++++ ...flake8_logging_format__tests__G001.py.snap | 15 ++ ...flake8_logging_format__tests__G002.py.snap | 15 ++ ...flake8_logging_format__tests__G003.py.snap | 15 ++ ...flake8_logging_format__tests__G004.py.snap | 15 ++ ...flake8_logging_format__tests__G010.py.snap | 22 ++ ...ake8_logging_format__tests__G101_1.py.snap | 15 ++ ...ake8_logging_format__tests__G101_2.py.snap | 15 ++ ...flake8_logging_format__tests__G201.py.snap | 15 ++ ...flake8_logging_format__tests__G202.py.snap | 15 ++ ..._tests__G_argparse_parser_error_ok.py.snap | 6 + ..._logging_format__tests__G_extra_ok.py.snap | 6 + ...rmat__tests__G_extra_str_format_ok.py.snap | 6 + ...logging_format__tests__G_simple_ok.py.snap | 6 + ...gging_format__tests__G_warnings_ok.py.snap | 6 + src/rules/flake8_logging_format/violations.rs | 91 +++++++ src/rules/mod.rs | 1 + .../rules/error_instead_of_exception.rs | 39 ++- 39 files changed, 921 insertions(+), 26 deletions(-) create mode 100644 resources/test/fixtures/flake8_logging_format/G001.py create mode 100644 resources/test/fixtures/flake8_logging_format/G002.py create mode 100644 resources/test/fixtures/flake8_logging_format/G003.py create mode 100644 resources/test/fixtures/flake8_logging_format/G004.py create mode 100644 resources/test/fixtures/flake8_logging_format/G010.py create mode 100644 resources/test/fixtures/flake8_logging_format/G101_1.py create mode 100644 resources/test/fixtures/flake8_logging_format/G101_2.py create mode 100644 resources/test/fixtures/flake8_logging_format/G201.py create mode 100644 resources/test/fixtures/flake8_logging_format/G202.py create mode 100644 resources/test/fixtures/flake8_logging_format/G_argparse_parser_error_ok.py create mode 100644 resources/test/fixtures/flake8_logging_format/G_extra_ok.py create mode 100644 resources/test/fixtures/flake8_logging_format/G_extra_str_format_ok.py create mode 100644 resources/test/fixtures/flake8_logging_format/G_simple_ok.py create mode 100644 resources/test/fixtures/flake8_logging_format/G_warnings_ok.py create mode 100644 src/rules/flake8_logging_format/mod.rs create mode 100644 src/rules/flake8_logging_format/rules.rs create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G002.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G003.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G004.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G010.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G201.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G202.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_argparse_parser_error_ok.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_extra_ok.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_extra_str_format_ok.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_simple_ok.py.snap create mode 100644 src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_warnings_ok.py.snap create mode 100644 src/rules/flake8_logging_format/violations.rs diff --git a/LICENSE b/LICENSE index 2c33a52682..7811d42465 100644 --- a/LICENSE +++ b/LICENSE @@ -800,3 +800,208 @@ are: OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ + +- flake8-logging-format, licensed as follows: + """ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "{}" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright {yyyy} {name of copyright owner} + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + """ diff --git a/README.md b/README.md index 032990bdf5..97a3751681 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,7 @@ developer of [Zulip](https://github.com/zulip/zulip): 1. [flake8-type-checking (TCH)](#flake8-type-checking-tch) 1. [tryceratops (TRY)](#tryceratops-try) 1. [flake8-use-pathlib (PTH)](#flake8-use-pathlib-pth) + 1. [flake8-logging-format (G)](#flake8-logging-format-g) 1. [Ruff-specific rules (RUF)](#ruff-specific-rules-ruf) 1. [Editor Integrations](#editor-integrations) 1. [FAQ](#faq) @@ -1257,6 +1258,21 @@ For more, see [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/) | PTH123 | pathlib-open | `open("foo")` should be replaced by`Path("foo").open()` | | | PTH124 | pathlib-py-path | `py.path` is in maintenance mode, use `pathlib` instead | | +### flake8-logging-format (G) + +For more, see [flake8-logging-format](https://pypi.org/project/flake8-logging-format/0.9.0/) on PyPI. + +| Code | Name | Message | Fix | +| ---- | ---- | ------- | --- | +| G001 | logging-string-format | Logging statement uses `string.format()` | | +| G002 | logging-percent-format | Logging statement uses `%` | | +| G003 | logging-string-concat | Logging statement uses `+` | | +| G004 | logging-f-string | Logging statement uses f-string | | +| G010 | logging-warn | Logging statement uses `warn` instead of `warning` | 🛠 | +| G101 | logging-extra-attr-clash | Logging statement uses an extra field that clashes with a LogRecord field: `{key}` | | +| G201 | logging-exc-info | Logging `.exception(...)` should be used instead of `.error(..., exc_info=True)` | | +| G202 | logging-redundant-exc-info | Logging statement has redundant `exc_info` | | + ### Ruff-specific rules (RUF) | Code | Name | Message | Fix | @@ -1552,6 +1568,7 @@ natively, including: - [`flake8-executable`](https://pypi.org/project/flake8-executable/) - [`flake8-implicit-str-concat`](https://pypi.org/project/flake8-implicit-str-concat/) - [`flake8-import-conventions`](https://github.com/joaopalmeiro/flake8-import-conventions) +- [`flake8-logging-format](https://pypi.org/project/flake8-logging-format/) - [`flake8-no-pep420`](https://pypi.org/project/flake8-no-pep420) - [`flake8-pie`](https://pypi.org/project/flake8-pie/) ([#1543](https://github.com/charliermarsh/ruff/issues/1543)) - [`flake8-print`](https://pypi.org/project/flake8-print/) @@ -1561,6 +1578,8 @@ natively, including: - [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) ([#998](https://github.com/charliermarsh/ruff/issues/998)) - [`flake8-super`](https://pypi.org/project/flake8-super/) - [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) +- [`flake8-type-checking](https://pypi.org/project/flake8-type-checking/) +- [`flake8-use-pathlib`](https://pypi.org/project/flake8-use-pathlib/) - [`isort`](https://pypi.org/project/isort/) - [`mccabe`](https://pypi.org/project/mccabe/) - [`pandas-vet`](https://pypi.org/project/pandas-vet/) @@ -1622,6 +1641,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [`flake8-executable`](https://pypi.org/project/flake8-executable/) - [`flake8-implicit-str-concat`](https://pypi.org/project/flake8-implicit-str-concat/) - [`flake8-import-conventions`](https://github.com/joaopalmeiro/flake8-import-conventions) +- [`flake8-logging-format](https://pypi.org/project/flake8-logging-format/) - [`flake8-no-pep420`](https://pypi.org/project/flake8-no-pep420) - [`flake8-pie`](https://pypi.org/project/flake8-pie/) ([#1543](https://github.com/charliermarsh/ruff/issues/1543)) - [`flake8-print`](https://pypi.org/project/flake8-print/) @@ -1631,6 +1651,8 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [`flake8-simplify`](https://pypi.org/project/flake8-simplify/) ([#998](https://github.com/charliermarsh/ruff/issues/998)) - [`flake8-super`](https://pypi.org/project/flake8-super/) - [`flake8-tidy-imports`](https://pypi.org/project/flake8-tidy-imports/) +- [`flake8-type-checking](https://pypi.org/project/flake8-type-checking/) +- [`flake8-use-pathlib`](https://pypi.org/project/flake8-use-pathlib/) - [`mccabe`](https://pypi.org/project/mccabe/) - [`pandas-vet`](https://pypi.org/project/pandas-vet/) - [`pep8-naming`](https://pypi.org/project/pep8-naming/) diff --git a/resources/test/fixtures/flake8_logging_format/G001.py b/resources/test/fixtures/flake8_logging_format/G001.py new file mode 100644 index 0000000000..2c3e0bc7d2 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G001.py @@ -0,0 +1,3 @@ +import logging + +logging.info("Hello {}".format("World!")) diff --git a/resources/test/fixtures/flake8_logging_format/G002.py b/resources/test/fixtures/flake8_logging_format/G002.py new file mode 100644 index 0000000000..0d2cbb7c61 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G002.py @@ -0,0 +1,3 @@ +import logging + +logging.info("Hello %s" % "World!") diff --git a/resources/test/fixtures/flake8_logging_format/G003.py b/resources/test/fixtures/flake8_logging_format/G003.py new file mode 100644 index 0000000000..7a1c6ae821 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G003.py @@ -0,0 +1,3 @@ +import logging + +logging.info("Hello" + " " + "World!") diff --git a/resources/test/fixtures/flake8_logging_format/G004.py b/resources/test/fixtures/flake8_logging_format/G004.py new file mode 100644 index 0000000000..e8f45eb72d --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G004.py @@ -0,0 +1,4 @@ +import logging + +name = "world" +logging.info(f"Hello {name}") diff --git a/resources/test/fixtures/flake8_logging_format/G010.py b/resources/test/fixtures/flake8_logging_format/G010.py new file mode 100644 index 0000000000..9b39f595a5 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G010.py @@ -0,0 +1,3 @@ +import logging + +logging.warn("Hello World!") diff --git a/resources/test/fixtures/flake8_logging_format/G101_1.py b/resources/test/fixtures/flake8_logging_format/G101_1.py new file mode 100644 index 0000000000..3386f5e480 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G101_1.py @@ -0,0 +1,8 @@ +import logging + +logging.info( + "Hello world!", + extra={ + "name": "foobar", + }, +) diff --git a/resources/test/fixtures/flake8_logging_format/G101_2.py b/resources/test/fixtures/flake8_logging_format/G101_2.py new file mode 100644 index 0000000000..c73c54e4c6 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G101_2.py @@ -0,0 +1,8 @@ +import logging + +logging.info( + "Hello world!", + extra=dict( + name="foobar", + ), +) diff --git a/resources/test/fixtures/flake8_logging_format/G201.py b/resources/test/fixtures/flake8_logging_format/G201.py new file mode 100644 index 0000000000..130d977db6 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G201.py @@ -0,0 +1,3 @@ +import logging + +logging.error('Hello World', exc_info=True) diff --git a/resources/test/fixtures/flake8_logging_format/G202.py b/resources/test/fixtures/flake8_logging_format/G202.py new file mode 100644 index 0000000000..16d9c27bb4 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G202.py @@ -0,0 +1,3 @@ +import logging + +logging.exception('Hello World', exc_info=True) diff --git a/resources/test/fixtures/flake8_logging_format/G_argparse_parser_error_ok.py b/resources/test/fixtures/flake8_logging_format/G_argparse_parser_error_ok.py new file mode 100644 index 0000000000..075cb39c41 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G_argparse_parser_error_ok.py @@ -0,0 +1,7 @@ +import argparse +from pathlib import Path + +parser = argparse.ArgumentParser() +parser.add_argument("target_dir", type=Path) +args = parser.parse_args() +parser.error(f"Target directory {args.target_dir} does not exist") diff --git a/resources/test/fixtures/flake8_logging_format/G_extra_ok.py b/resources/test/fixtures/flake8_logging_format/G_extra_ok.py new file mode 100644 index 0000000000..17eb33a673 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G_extra_ok.py @@ -0,0 +1,8 @@ +import logging + +logging.info( + "Hello {world}!", + extra=dict( + world="World", + ), +) diff --git a/resources/test/fixtures/flake8_logging_format/G_extra_str_format_ok.py b/resources/test/fixtures/flake8_logging_format/G_extra_str_format_ok.py new file mode 100644 index 0000000000..04b1d09e64 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G_extra_str_format_ok.py @@ -0,0 +1,8 @@ +import logging + +logging.info( + "Hello {world}!", + extra=dict( + world="{}".format("World"), + ), +) diff --git a/resources/test/fixtures/flake8_logging_format/G_simple_ok.py b/resources/test/fixtures/flake8_logging_format/G_simple_ok.py new file mode 100644 index 0000000000..ca9b6b7aa8 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G_simple_ok.py @@ -0,0 +1,3 @@ +import logging + +logging.info("Hello World!") diff --git a/resources/test/fixtures/flake8_logging_format/G_warnings_ok.py b/resources/test/fixtures/flake8_logging_format/G_warnings_ok.py new file mode 100644 index 0000000000..9e48283ee1 --- /dev/null +++ b/resources/test/fixtures/flake8_logging_format/G_warnings_ok.py @@ -0,0 +1,3 @@ +import warnings + +warnings.warn("Hello World!") diff --git a/ruff.schema.json b/ruff.schema.json index 66c0bfc55b..a0ee6727b6 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1475,6 +1475,22 @@ "FBT001", "FBT002", "FBT003", + "G", + "G0", + "G00", + "G001", + "G002", + "G003", + "G004", + "G01", + "G010", + "G1", + "G10", + "G101", + "G2", + "G20", + "G201", + "G202", "I", "I0", "I00", diff --git a/src/ast/helpers.rs b/src/ast/helpers.rs index 86de087f28..9ebad0d4ba 100644 --- a/src/ast/helpers.rs +++ b/src/ast/helpers.rs @@ -974,6 +974,21 @@ impl<'a> SimpleCallArgs<'a> { } } +/// Return `true` if the given `Expr` is a potential logging call. Matches +/// `logging.error`, `logger.error`, `self.logger.error`, etc., but not +/// arbitrary `foo.error` calls. +pub fn is_logger_candidate(func: &Expr) -> bool { + if let ExprKind::Attribute { value, .. } = &func.node { + let call_path = collect_call_path(value); + if let Some(tail) = call_path.last() { + if *tail == "logging" || tail.ends_with("logger") { + return true; + } + } + } + false +} + #[cfg(test)] mod tests { use anyhow::Result; diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 7d08cdb694..0199c52219 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -34,10 +34,11 @@ use crate::registry::{Diagnostic, Rule}; use crate::rules::{ flake8_2020, flake8_annotations, flake8_bandit, flake8_blind_except, flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, flake8_debugger, - flake8_errmsg, flake8_implicit_str_concat, flake8_import_conventions, flake8_pie, flake8_print, - flake8_pytest_style, flake8_return, flake8_simplify, flake8_tidy_imports, flake8_type_checking, - flake8_unused_arguments, flake8_use_pathlib, mccabe, pandas_vet, pep8_naming, pycodestyle, - pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, ruff, tryceratops, + flake8_errmsg, flake8_implicit_str_concat, flake8_import_conventions, flake8_logging_format, + flake8_pie, flake8_print, flake8_pytest_style, flake8_return, flake8_simplify, + flake8_tidy_imports, flake8_type_checking, flake8_unused_arguments, flake8_use_pathlib, mccabe, + pandas_vet, pep8_naming, pycodestyle, pydocstyle, pyflakes, pygrep_hooks, pylint, pyupgrade, + ruff, tryceratops, }; use crate::settings::types::PythonVersion; use crate::settings::{flags, Settings}; @@ -2655,6 +2656,19 @@ where { flake8_use_pathlib::helpers::replaceable_by_pathlib(self, func); } + + // flake8-logging-format + if self.settings.rules.enabled(&Rule::LoggingStringFormat) + || self.settings.rules.enabled(&Rule::LoggingPercentFormat) + || self.settings.rules.enabled(&Rule::LoggingStringConcat) + || self.settings.rules.enabled(&Rule::LoggingFString) + || self.settings.rules.enabled(&Rule::LoggingWarn) + || self.settings.rules.enabled(&Rule::LoggingExtraAttrClash) + || self.settings.rules.enabled(&Rule::LoggingExcInfo) + || self.settings.rules.enabled(&Rule::LoggingRedundantExcInfo) + { + flake8_logging_format::rules::logging_call(self, func, args, keywords); + } } ExprKind::Dict { keys, values } => { if self diff --git a/src/registry.rs b/src/registry.rs index 4f4a73730c..0a0423e5a9 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -469,6 +469,15 @@ ruff_macros::define_rule_mapping!( PTH122 => rules::flake8_use_pathlib::violations::PathlibSplitext, PTH123 => rules::flake8_use_pathlib::violations::PathlibOpen, PTH124 => rules::flake8_use_pathlib::violations::PathlibPyPath, + // flake8-logging-format + G001 => rules::flake8_logging_format::violations::LoggingStringFormat, + G002 => rules::flake8_logging_format::violations::LoggingPercentFormat, + G003 => rules::flake8_logging_format::violations::LoggingStringConcat, + G004 => rules::flake8_logging_format::violations::LoggingFString, + G010 => rules::flake8_logging_format::violations::LoggingWarn, + G101 => rules::flake8_logging_format::violations::LoggingExtraAttrClash, + G201 => rules::flake8_logging_format::violations::LoggingExcInfo, + G202 => rules::flake8_logging_format::violations::LoggingRedundantExcInfo, // ruff RUF001 => violations::AmbiguousUnicodeCharacterString, RUF002 => violations::AmbiguousUnicodeCharacterDocstring, @@ -595,6 +604,9 @@ pub enum Linter { /// [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/) #[prefix = "PTH"] Flake8UsePathlib, + /// [flake8-logging-format](https://pypi.org/project/flake8-logging-format/0.9.0/) + #[prefix = "G"] + Flake8LoggingFormat, /// Ruff-specific rules #[prefix = "RUF"] Ruff, diff --git a/src/rules/flake8_logging_format/mod.rs b/src/rules/flake8_logging_format/mod.rs new file mode 100644 index 0000000000..d07acc26d7 --- /dev/null +++ b/src/rules/flake8_logging_format/mod.rs @@ -0,0 +1,50 @@ +//! Rules from [flake8-logging-format](https://pypi.org/project/flake8-logging-format/0.9.0/). +pub(crate) mod rules; +pub(crate) mod violations; + +#[cfg(test)] +mod tests { + use std::path::Path; + + use anyhow::Result; + use test_case::test_case; + + use crate::linter::test_path; + use crate::registry::Rule; + use crate::settings; + + #[test_case(Path::new("G_argparse_parser_error_ok.py"); "G_argparse_parser_error_ok")] + #[test_case(Path::new("G_extra_ok.py"); "G_extra_ok")] + #[test_case(Path::new("G_extra_str_format_ok.py"); "G_extra_str_format_ok")] + #[test_case(Path::new("G_simple_ok.py"); "G_simple_ok")] + #[test_case(Path::new("G_warnings_ok.py"); "G_warnings_ok")] + #[test_case(Path::new("G001.py"); "G001")] + #[test_case(Path::new("G002.py"); "G002")] + #[test_case(Path::new("G003.py"); "G003")] + #[test_case(Path::new("G004.py"); "G004")] + #[test_case(Path::new("G010.py"); "G010")] + #[test_case(Path::new("G101_1.py"); "G101_1")] + #[test_case(Path::new("G101_2.py"); "G101_2")] + #[test_case(Path::new("G201.py"); "G201")] + #[test_case(Path::new("G202.py"); "G202")] + fn rules(path: &Path) -> Result<()> { + let snapshot = path.to_string_lossy().into_owned(); + let diagnostics = test_path( + Path::new("./resources/test/fixtures/flake8_logging_format") + .join(path) + .as_path(), + &settings::Settings::for_rules(vec![ + Rule::LoggingStringFormat, + Rule::LoggingPercentFormat, + Rule::LoggingStringConcat, + Rule::LoggingFString, + Rule::LoggingWarn, + Rule::LoggingExtraAttrClash, + Rule::LoggingExcInfo, + Rule::LoggingRedundantExcInfo, + ]), + )?; + insta::assert_yaml_snapshot!(snapshot, diagnostics); + Ok(()) + } +} diff --git a/src/rules/flake8_logging_format/rules.rs b/src/rules/flake8_logging_format/rules.rs new file mode 100644 index 0000000000..2651da293a --- /dev/null +++ b/src/rules/flake8_logging_format/rules.rs @@ -0,0 +1,235 @@ +use rustpython_ast::{Constant, Expr, ExprKind, Keyword, Operator}; +use rustpython_parser::ast::Location; + +use crate::ast::helpers::{find_keyword, is_logger_candidate, SimpleCallArgs}; +use crate::ast::types::Range; +use crate::checkers::ast::Checker; +use crate::fix::Fix; +use crate::registry::{Diagnostic, Rule}; +use crate::rules::flake8_logging_format::violations::{ + LoggingExcInfo, LoggingExtraAttrClash, LoggingFString, LoggingPercentFormat, + LoggingRedundantExcInfo, LoggingStringConcat, LoggingStringFormat, LoggingWarn, +}; + +enum LoggingLevel { + Debug, + Critical, + Error, + Exception, + Info, + Warn, + Warning, +} + +impl LoggingLevel { + fn from_str(level: &str) -> Option { + match level { + "debug" => Some(LoggingLevel::Debug), + "critical" => Some(LoggingLevel::Critical), + "error" => Some(LoggingLevel::Error), + "exception" => Some(LoggingLevel::Exception), + "info" => Some(LoggingLevel::Info), + "warn" => Some(LoggingLevel::Warn), + "warning" => Some(LoggingLevel::Warning), + _ => None, + } + } +} + +const RESERVED_ATTRS: &[&str; 22] = &[ + "args", + "asctime", + "created", + "exc_info", + "exc_text", + "filename", + "funcName", + "levelname", + "levelno", + "lineno", + "module", + "msecs", + "message", + "msg", + "name", + "pathname", + "process", + "processName", + "relativeCreated", + "stack_info", + "thread", + "threadName", +]; + +/// Check logging messages for violations. +fn check_msg(checker: &mut Checker, msg: &Expr) { + match &msg.node { + // Check for string concatenation and percent format. + ExprKind::BinOp { op, .. } => match op { + Operator::Add => { + if checker.settings.rules.enabled(&Rule::LoggingStringConcat) { + checker.diagnostics.push(Diagnostic::new( + LoggingStringConcat, + Range::from_located(msg), + )); + } + } + Operator::Mod => { + if checker.settings.rules.enabled(&Rule::LoggingPercentFormat) { + checker.diagnostics.push(Diagnostic::new( + LoggingPercentFormat, + Range::from_located(msg), + )); + } + } + _ => {} + }, + // Check for f-strings. + ExprKind::JoinedStr { .. } => { + if checker.settings.rules.enabled(&Rule::LoggingFString) { + checker + .diagnostics + .push(Diagnostic::new(LoggingFString, Range::from_located(msg))); + } + } + // Check for .format() calls. + ExprKind::Call { func, .. } => { + if checker.settings.rules.enabled(&Rule::LoggingStringFormat) { + if let ExprKind::Attribute { value, attr, .. } = &func.node { + if attr == "format" && matches!(value.node, ExprKind::Constant { .. }) { + checker.diagnostics.push(Diagnostic::new( + LoggingStringFormat, + Range::from_located(msg), + )); + } + } + } + } + _ => {} + } +} + +/// Check contents of the `extra` argument to logging calls. +fn check_log_record_attr_clash(checker: &mut Checker, extra: &Keyword) { + match &extra.node.value.node { + ExprKind::Dict { keys, .. } => { + for key in keys { + if let Some(key) = &key { + if let ExprKind::Constant { + value: Constant::Str(string), + .. + } = &key.node + { + if RESERVED_ATTRS.contains(&string.as_str()) { + checker.diagnostics.push(Diagnostic::new( + LoggingExtraAttrClash(string.to_string()), + Range::from_located(key), + )); + } + } + } + } + } + ExprKind::Call { func, keywords, .. } => { + if checker + .resolve_call_path(func) + .map_or(false, |call_path| call_path.as_slice() == ["", "dict"]) + { + for keyword in keywords { + if let Some(key) = &keyword.node.arg { + if RESERVED_ATTRS.contains(&key.as_str()) { + checker.diagnostics.push(Diagnostic::new( + LoggingExtraAttrClash(key.to_string()), + Range::from_located(keyword), + )); + } + } + } + } + } + _ => {} + } +} + +/// Check logging calls for violations. +pub fn logging_call(checker: &mut Checker, func: &Expr, args: &[Expr], keywords: &[Keyword]) { + if !is_logger_candidate(func) { + return; + } + + if let ExprKind::Attribute { value, attr, .. } = &func.node { + if let Some(logging_level) = LoggingLevel::from_str(attr.as_str()) { + let call_args = SimpleCallArgs::new(args, keywords); + let level_call_range = Range::new( + Location::new( + func.location.row(), + value.end_location.unwrap().column() + 1, + ), + Location::new( + func.end_location.unwrap().row(), + func.end_location.unwrap().column(), + ), + ); + + // G001 - G004 + if let Some(format_arg) = call_args.get_argument("msg", Some(0)) { + check_msg(checker, format_arg); + } + + // G010 + if checker.settings.rules.enabled(&Rule::LoggingWarn) + && matches!(logging_level, LoggingLevel::Warn) + { + let mut diagnostic = Diagnostic::new(LoggingWarn, level_call_range); + if checker.patch(diagnostic.kind.rule()) { + diagnostic.amend(Fix::replacement( + "warning".to_string(), + level_call_range.location, + level_call_range.end_location, + )); + } + checker.diagnostics.push(diagnostic); + } + + // G101 + if checker.settings.rules.enabled(&Rule::LoggingExtraAttrClash) { + if let Some(extra) = find_keyword(keywords, "extra") { + check_log_record_attr_clash(checker, extra); + } + } + + // G201, G202 + if checker.settings.rules.enabled(&Rule::LoggingExcInfo) + || checker + .settings + .rules + .enabled(&Rule::LoggingRedundantExcInfo) + { + if let Some(exc_info) = find_keyword(keywords, "exc_info") { + match logging_level { + LoggingLevel::Error => { + if checker.settings.rules.enabled(&Rule::LoggingExcInfo) { + checker + .diagnostics + .push(Diagnostic::new(LoggingExcInfo, level_call_range)); + } + } + LoggingLevel::Exception => { + if checker + .settings + .rules + .enabled(&Rule::LoggingRedundantExcInfo) + { + checker.diagnostics.push(Diagnostic::new( + LoggingRedundantExcInfo, + Range::from_located(exc_info), + )); + } + } + _ => {} + } + } + } + } + } +} diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap new file mode 100644 index 0000000000..c9bf425b68 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G001.py.snap @@ -0,0 +1,15 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingStringFormat: ~ + location: + row: 3 + column: 13 + end_location: + row: 3 + column: 40 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G002.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G002.py.snap new file mode 100644 index 0000000000..17354d07bf --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G002.py.snap @@ -0,0 +1,15 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingPercentFormat: ~ + location: + row: 3 + column: 13 + end_location: + row: 3 + column: 34 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G003.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G003.py.snap new file mode 100644 index 0000000000..913bb55d16 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G003.py.snap @@ -0,0 +1,15 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingStringConcat: ~ + location: + row: 3 + column: 13 + end_location: + row: 3 + column: 37 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G004.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G004.py.snap new file mode 100644 index 0000000000..7968690402 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G004.py.snap @@ -0,0 +1,15 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingFString: ~ + location: + row: 4 + column: 13 + end_location: + row: 4 + column: 28 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G010.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G010.py.snap new file mode 100644 index 0000000000..07d7a4c499 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G010.py.snap @@ -0,0 +1,22 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingWarn: ~ + location: + row: 3 + column: 8 + end_location: + row: 3 + column: 12 + fix: + content: warning + location: + row: 3 + column: 8 + end_location: + row: 3 + column: 12 + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap new file mode 100644 index 0000000000..3386ab2ee2 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_1.py.snap @@ -0,0 +1,15 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingExtraAttrClash: name + location: + row: 6 + column: 8 + end_location: + row: 6 + column: 14 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap new file mode 100644 index 0000000000..81f578136e --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G101_2.py.snap @@ -0,0 +1,15 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingExtraAttrClash: name + location: + row: 6 + column: 8 + end_location: + row: 6 + column: 21 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G201.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G201.py.snap new file mode 100644 index 0000000000..9738951e5b --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G201.py.snap @@ -0,0 +1,15 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingExcInfo: ~ + location: + row: 3 + column: 8 + end_location: + row: 3 + column: 13 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G202.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G202.py.snap new file mode 100644 index 0000000000..a75ddd23a0 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G202.py.snap @@ -0,0 +1,15 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +- kind: + LoggingRedundantExcInfo: ~ + location: + row: 3 + column: 33 + end_location: + row: 3 + column: 46 + fix: ~ + parent: ~ + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_argparse_parser_error_ok.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_argparse_parser_error_ok.py.snap new file mode 100644 index 0000000000..c3e0ede3d2 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_argparse_parser_error_ok.py.snap @@ -0,0 +1,6 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +[] + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_extra_ok.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_extra_ok.py.snap new file mode 100644 index 0000000000..c3e0ede3d2 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_extra_ok.py.snap @@ -0,0 +1,6 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +[] + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_extra_str_format_ok.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_extra_str_format_ok.py.snap new file mode 100644 index 0000000000..c3e0ede3d2 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_extra_str_format_ok.py.snap @@ -0,0 +1,6 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +[] + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_simple_ok.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_simple_ok.py.snap new file mode 100644 index 0000000000..c3e0ede3d2 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_simple_ok.py.snap @@ -0,0 +1,6 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +[] + diff --git a/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_warnings_ok.py.snap b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_warnings_ok.py.snap new file mode 100644 index 0000000000..c3e0ede3d2 --- /dev/null +++ b/src/rules/flake8_logging_format/snapshots/ruff__rules__flake8_logging_format__tests__G_warnings_ok.py.snap @@ -0,0 +1,6 @@ +--- +source: src/rules/flake8_logging_format/mod.rs +expression: diagnostics +--- +[] + diff --git a/src/rules/flake8_logging_format/violations.rs b/src/rules/flake8_logging_format/violations.rs new file mode 100644 index 0000000000..f2c65c4d75 --- /dev/null +++ b/src/rules/flake8_logging_format/violations.rs @@ -0,0 +1,91 @@ +use ruff_macros::derive_message_formats; + +use crate::define_violation; +use crate::violation::{AlwaysAutofixableViolation, Violation}; + +define_violation!( + pub struct LoggingStringFormat; +); +impl Violation for LoggingStringFormat { + #[derive_message_formats] + fn message(&self) -> String { + format!("Logging statement uses `string.format()`") + } +} + +define_violation!( + pub struct LoggingPercentFormat; +); +impl Violation for LoggingPercentFormat { + #[derive_message_formats] + fn message(&self) -> String { + format!("Logging statement uses `%`") + } +} + +define_violation!( + pub struct LoggingStringConcat; +); +impl Violation for LoggingStringConcat { + #[derive_message_formats] + fn message(&self) -> String { + format!("Logging statement uses `+`") + } +} + +define_violation!( + pub struct LoggingFString; +); +impl Violation for LoggingFString { + #[derive_message_formats] + fn message(&self) -> String { + format!("Logging statement uses f-string") + } +} + +define_violation!( + pub struct LoggingWarn; +); +impl AlwaysAutofixableViolation for LoggingWarn { + #[derive_message_formats] + fn message(&self) -> String { + format!("Logging statement uses `warn` instead of `warning`") + } + + fn autofix_title(&self) -> String { + "Convert to `warn`".to_string() + } +} + +define_violation!( + pub struct LoggingExtraAttrClash(pub String); +); +impl Violation for LoggingExtraAttrClash { + #[derive_message_formats] + fn message(&self) -> String { + let LoggingExtraAttrClash(key) = self; + format!( + "Logging statement uses an extra field that clashes with a LogRecord field: `{key}`" + ) + } +} + +define_violation!( + pub struct LoggingExcInfo; +); +impl Violation for LoggingExcInfo { + #[derive_message_formats] + fn message(&self) -> String { + format!("Logging `.exception(...)` should be used instead of `.error(..., exc_info=True)`") + } +} + +define_violation!( + pub struct LoggingRedundantExcInfo; +); +impl Violation for LoggingRedundantExcInfo { + #[derive_message_formats] + fn message(&self) -> String { + format!("Logging statement has redundant `exc_info`") + } +} diff --git a/src/rules/mod.rs b/src/rules/mod.rs index 8e58049101..30f9c28d25 100644 --- a/src/rules/mod.rs +++ b/src/rules/mod.rs @@ -15,6 +15,7 @@ pub mod flake8_errmsg; pub mod flake8_executable; pub mod flake8_implicit_str_concat; pub mod flake8_import_conventions; +pub mod flake8_logging_format; pub mod flake8_no_pep420; pub mod flake8_pie; pub mod flake8_print; diff --git a/src/rules/tryceratops/rules/error_instead_of_exception.rs b/src/rules/tryceratops/rules/error_instead_of_exception.rs index 6fc3c7ee73..fd070c199e 100644 --- a/src/rules/tryceratops/rules/error_instead_of_exception.rs +++ b/src/rules/tryceratops/rules/error_instead_of_exception.rs @@ -1,7 +1,7 @@ use ruff_macros::derive_message_formats; use rustpython_ast::{Excepthandler, ExcepthandlerKind, Expr, ExprKind}; -use crate::ast::helpers::collect_call_path; +use crate::ast::helpers::is_logger_candidate; use crate::ast::types::Range; use crate::ast::visitor; use crate::ast::visitor::Visitor; @@ -21,28 +21,19 @@ impl Violation for ErrorInsteadOfException { } #[derive(Default)] -/// Collect `logging.error`-like calls from an AST. Matches `logging.error`, -/// `logger.error`, `self.logger.error`, etc., but not arbitrary `foo.error` -/// calls. -struct ErrorCallVisitor<'a> { - calls: Vec<&'a Expr>, +/// Collect `logging`-like calls from an AST. +struct LoggerCandidateVisitor<'a> { + calls: Vec<(&'a Expr, &'a Expr)>, } -impl<'a, 'b> Visitor<'b> for ErrorCallVisitor<'a> +impl<'a, 'b> Visitor<'b> for LoggerCandidateVisitor<'a> where 'b: 'a, { fn visit_expr(&mut self, expr: &'b Expr) { if let ExprKind::Call { func, .. } = &expr.node { - if let ExprKind::Attribute { value, attr, .. } = &func.node { - if attr == "error" { - let call_path = collect_call_path(value); - if let Some(tail) = call_path.last() { - if *tail == "logging" || tail.ends_with("logger") { - self.calls.push(expr); - } - } - } + if is_logger_candidate(func) { + self.calls.push((expr, func)); } } visitor::walk_expr(self, expr); @@ -54,15 +45,19 @@ pub fn error_instead_of_exception(checker: &mut Checker, handlers: &[Excepthandl for handler in handlers { let ExcepthandlerKind::ExceptHandler { body, .. } = &handler.node; let calls = { - let mut visitor = ErrorCallVisitor::default(); + let mut visitor = LoggerCandidateVisitor::default(); visitor.visit_body(body); visitor.calls }; - for expr in calls { - checker.diagnostics.push(Diagnostic::new( - ErrorInsteadOfException, - Range::from_located(expr), - )); + for (expr, func) in calls { + if let ExprKind::Attribute { attr, .. } = &func.node { + if attr == "error" { + checker.diagnostics.push(Diagnostic::new( + ErrorInsteadOfException, + Range::from_located(expr), + )); + } + } } } }