From d098118e37be9437345d527c29a5aeea91b676a6 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 20 May 2025 14:47:56 -0400 Subject: [PATCH] [ty] disable division-by-zero by default (#18220) ## Summary I think `division-by-zero` is a low-value diagnostic in general; most real division-by-zero errors (especially those that are less obvious to the human eye) will occur on values typed as `int`, in which case we don't issue the diagnostic anyway. Mypy and pyright do not emit this diagnostic. Currently the diagnostic is prone to false positives because a) we do not silence it in unreachable code, and b) we do not implement narrowing of literals from inequality checks. We will probably fix (a) regardless, but (b) is low priority apart from division-by-zero. I think we have many more important things to do and should not allow false positives on a low-value diagnostic to be a distraction. Not opposed to re-enabling this diagnostic in future when we can prioritize reducing its false positives. References https://github.com/astral-sh/ty/issues/443 ## Test Plan Existing tests. --- .github/mypy-primer-ty.toml | 1 + crates/ty/docs/rules.md | 46 +++++------ crates/ty/tests/cli.rs | 77 ++++--------------- .../src/types/diagnostic.rs | 2 +- ty.schema.json | 2 +- 5 files changed, 43 insertions(+), 85 deletions(-) diff --git a/.github/mypy-primer-ty.toml b/.github/mypy-primer-ty.toml index 0e8e75303e..9317364fe2 100644 --- a/.github/mypy-primer-ty.toml +++ b/.github/mypy-primer-ty.toml @@ -5,3 +5,4 @@ [rules] possibly-unresolved-reference = "warn" unused-ignore-comment = "warn" +division-by-zero = "warn" diff --git a/crates/ty/docs/rules.md b/crates/ty/docs/rules.md index 7772a0b1d1..5acf281cce 100644 --- a/crates/ty/docs/rules.md +++ b/crates/ty/docs/rules.md @@ -176,29 +176,6 @@ class B(A): ... * [View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L195) -## `division-by-zero` - -**Default level**: error - -
-detects division by zero - -### What it does -It detects division by zero. - -### Why is this bad? -Dividing by zero raises a `ZeroDivisionError` at runtime. - -### Examples -```python -5 / 0 -``` - -### Links -* [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20division-by-zero) -* [View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L221) -
- ## `duplicate-base` **Default level**: error @@ -1743,6 +1720,29 @@ a = 20 / 0 # ty: ignore[division-by-zero] * [View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Fsuppression.rs#L40) +## `division-by-zero` + +**Default level**: ignore + +
+detects division by zero + +### What it does +It detects division by zero. + +### Why is this bad? +Dividing by zero raises a `ZeroDivisionError` at runtime. + +### Examples +```python +5 / 0 +``` + +### Links +* [Related issues](https://github.com/astral-sh/ty/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20division-by-zero) +* [View source](https://github.com/astral-sh/ruff/blob/main/crates%2Fty_python_semantic%2Fsrc%2Ftypes%2Fdiagnostic.rs#L221) +
+ ## `possibly-unresolved-reference` **Default level**: ignore diff --git a/crates/ty/tests/cli.rs b/crates/ty/tests/cli.rs index f2d870744a..f0e7e2c0ba 100644 --- a/crates/ty/tests/cli.rs +++ b/crates/ty/tests/cli.rs @@ -387,22 +387,11 @@ fn configuration_rule_severity() -> anyhow::Result<()> { "#, )?; - // Assert that there's an `unresolved-reference` diagnostic (error) - // and a `division-by-zero` diagnostic (error). - assert_cmd_snapshot!(case.command(), @r" + // Assert that there's an `unresolved-reference` diagnostic (error). + assert_cmd_snapshot!(case.command(), @r###" success: false exit_code: 1 ----- stdout ----- - error[division-by-zero]: Cannot divide object of type `Literal[4]` by zero - --> test.py:2:5 - | - 2 | y = 4 / 0 - | ^^^^^ - 3 | - 4 | for a in range(0, int(y)): - | - info: rule `division-by-zero` is enabled by default - error[unresolved-reference]: Name `prin` used when not defined --> test.py:7:1 | @@ -413,17 +402,17 @@ fn configuration_rule_severity() -> anyhow::Result<()> { | info: rule `unresolved-reference` is enabled by default - Found 2 diagnostics + Found 1 diagnostic ----- stderr ----- WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. - "); + "###); case.write_file( "pyproject.toml", r#" [tool.ty.rules] - division-by-zero = "warn" # demote to warn + division-by-zero = "warn" # promote to warn unresolved-reference = "ignore" "#, )?; @@ -468,9 +457,9 @@ fn cli_rule_severity() -> anyhow::Result<()> { "#, )?; - // Assert that there's an `unresolved-reference` diagnostic (error), - // a `division-by-zero` (error) and a unresolved-import (error) diagnostic by default. - assert_cmd_snapshot!(case.command(), @r" + // Assert that there's an `unresolved-reference` diagnostic (error) + // and an unresolved-import (error) diagnostic by default. + assert_cmd_snapshot!(case.command(), @r###" success: false exit_code: 1 ----- stdout ----- @@ -485,18 +474,6 @@ fn cli_rule_severity() -> anyhow::Result<()> { info: make sure your Python environment is properly configured: https://github.com/astral-sh/ty/blob/main/docs/README.md#python-environment info: rule `unresolved-import` is enabled by default - error[division-by-zero]: Cannot divide object of type `Literal[4]` by zero - --> test.py:4:5 - | - 2 | import does_not_exit - 3 | - 4 | y = 4 / 0 - | ^^^^^ - 5 | - 6 | for a in range(0, int(y)): - | - info: rule `division-by-zero` is enabled by default - error[unresolved-reference]: Name `prin` used when not defined --> test.py:9:1 | @@ -507,11 +484,11 @@ fn cli_rule_severity() -> anyhow::Result<()> { | info: rule `unresolved-reference` is enabled by default - Found 3 diagnostics + Found 2 diagnostics ----- stderr ----- WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. - "); + "###); assert_cmd_snapshot!( case @@ -575,22 +552,11 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> { "#, )?; - // Assert that there's a `unresolved-reference` diagnostic (error) - // and a `division-by-zero` (error) by default. - assert_cmd_snapshot!(case.command(), @r" + // Assert that there's a `unresolved-reference` diagnostic (error) by default. + assert_cmd_snapshot!(case.command(), @r###" success: false exit_code: 1 ----- stdout ----- - error[division-by-zero]: Cannot divide object of type `Literal[4]` by zero - --> test.py:2:5 - | - 2 | y = 4 / 0 - | ^^^^^ - 3 | - 4 | for a in range(0, int(y)): - | - info: rule `division-by-zero` is enabled by default - error[unresolved-reference]: Name `prin` used when not defined --> test.py:7:1 | @@ -601,11 +567,11 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> { | info: rule `unresolved-reference` is enabled by default - Found 2 diagnostics + Found 1 diagnostic ----- stderr ----- WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. - "); + "###); assert_cmd_snapshot!( case @@ -614,7 +580,6 @@ fn cli_rule_severity_precedence() -> anyhow::Result<()> { .arg("unresolved-reference") .arg("--warn") .arg("division-by-zero") - // Override the error severity with warning .arg("--ignore") .arg("unresolved-reference"), @r" @@ -1103,18 +1068,10 @@ fn check_specific_paths() -> anyhow::Result<()> { assert_cmd_snapshot!( case.command(), - @r" + @r###" success: false exit_code: 1 ----- stdout ----- - error[division-by-zero]: Cannot divide object of type `Literal[4]` by zero - --> project/main.py:2:5 - | - 2 | y = 4 / 0 # error: division-by-zero - | ^^^^^ - | - info: rule `division-by-zero` is enabled by default - error[unresolved-import]: Cannot resolve imported module `main2` --> project/other.py:2:6 | @@ -1135,11 +1092,11 @@ fn check_specific_paths() -> anyhow::Result<()> { info: make sure your Python environment is properly configured: https://github.com/astral-sh/ty/blob/main/docs/README.md#python-environment info: rule `unresolved-import` is enabled by default - Found 3 diagnostics + Found 2 diagnostics ----- stderr ----- WARN ty is pre-release software and not ready for production use. Expect to encounter bugs, missing features, and fatal errors. - " + "### ); // Now check only the `tests` and `other.py` files. diff --git a/crates/ty_python_semantic/src/types/diagnostic.rs b/crates/ty_python_semantic/src/types/diagnostic.rs index b6a1e17be5..a9e6c6075c 100644 --- a/crates/ty_python_semantic/src/types/diagnostic.rs +++ b/crates/ty_python_semantic/src/types/diagnostic.rs @@ -232,7 +232,7 @@ declare_lint! { pub(crate) static DIVISION_BY_ZERO = { summary: "detects division by zero", status: LintStatus::preview("1.0.0"), - default_level: Level::Error, + default_level: Level::Ignore, } } diff --git a/ty.schema.json b/ty.schema.json index 0dae961d6d..636912e0bb 100644 --- a/ty.schema.json +++ b/ty.schema.json @@ -293,7 +293,7 @@ "division-by-zero": { "title": "detects division by zero", "description": "## What it does\nIt detects division by zero.\n\n## Why is this bad?\nDividing by zero raises a `ZeroDivisionError` at runtime.\n\n## Examples\n```python\n5 / 0\n```", - "default": "error", + "default": "ignore", "oneOf": [ { "$ref": "#/definitions/Level"