Skip to content

Add clint rule to flag except X or Y boolean operator misuse#20905

Merged
harupy merged 4 commits intomasterfrom
copilot/add-clint-rule-except-pattern
Feb 17, 2026
Merged

Add clint rule to flag except X or Y boolean operator misuse#20905
harupy merged 4 commits intomasterfrom
copilot/add-clint-rule-except-pattern

Conversation

Copy link
Contributor

Copilot AI commented Feb 17, 2026

Related Issues/PRs

What changes are proposed in this pull request?

Python's except ValueError or KeyError: is a common mistake—the or operator evaluates at parse time, returning the first truthy operand (ValueError), causing subsequent exception types to be silently ignored. The correct syntax is except (ValueError, KeyError):.

This PR adds a new clint rule except-bool-op (MLF0006) to detect this pattern:

Rule implementation

  • dev/clint/src/clint/rules/except_bool_op.py: AST-based check for BoolOp nodes in ExceptHandler.type
  • Detects both or and and operators, including chained cases like except X or Y or Z
  • Does not flag correct forms: except (X, Y), except X, or bare except:
  • Error message: "Did you mean except (X, Y):? Using or/and in an except handler is likely a mistake."

Linter integration

  • Added visit_ExceptHandler method to Linter class
  • Registered rule in rules/__init__.py

Example detection

# Flagged ❌
except ValueError or KeyError:
    pass

# Correct ✅
except (ValueError, KeyError):
    pass

The rule currently flags one existing instance at mlflow/exceptions.py:136 (being fixed in #20903).

How is this PR tested?

  • New unit/integration tests

Added dev/clint/tests/rules/test_except_bool_op.py with:

  • Positive cases: or, and, and chained operators
  • Negative cases: tuple syntax, single exceptions, bare except
  • Line number accuracy verification

All 236 clint tests pass.

Does this PR require documentation update?

  • No. You can skip the rest of this section.

Does this PR require updating the MLflow Skills repository?

  • No. You can skip the rest of this section.

Release Notes

Is this a user-facing change?

  • No. You can skip the rest of this section.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

  • area/build: Build and test infrastructure for MLflow

How should the PR be classified in the release notes? Choose one:

  • rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section

Should this PR be included in the next patch release?

  • Yes (this PR will be cherry-picked and included in the next patch release)
  • No (this PR will be included in the next minor release)
Original prompt

Add clint rule to flag except X or Y

Created by Claude Code for handoff.

Patch release: No

Summary

except ValueError or KeyError: is a Python bug — or evaluates at parse time and returns the first truthy operand (ValueError), so KeyError is silently ignored. The correct form is except (ValueError, KeyError):. Add a clint rule to catch this pattern using AST analysis.

Currently one instance exists in the codebase at mlflow/exceptions.py:136 (being fixed separately in PR #20903).

Codebase Analysis

  • dev/clint/src/clint/rules/base.py - Rule base class. Subclasses get auto-generated id (MLF####) and name (kebab-case from class name)
  • dev/clint/src/clint/rules/__init__.py - Rule registry. Import the new rule here and add to __all__
  • dev/clint/src/clint/linter.py - Main linter. No visit_ExceptHandler exists yet — add one to invoke the new rule
  • dev/clint/src/clint/rules/isinstance_union_syntax.py - Good pattern to follow (simple static check on a single node)
  • dev/clint/tests/rules/test_isinstance_union_syntax.py - Good test pattern to follow

Success Criteria

  • New rule file dev/clint/src/clint/rules/except_bool_op.py with ExceptBoolOp class
  • Rule flags except X or Y, except X and Y, and nested variants (e.g., except X or Y or Z)
  • Rule does NOT flag correct forms: except (X, Y), except X, bare except:
  • Rule registered in rules/__init__.py
  • visit_ExceptHandler added to Linter in linter.py
  • Test file dev/clint/tests/rules/test_except_bool_op.py with positive and negative cases
  • uv run clint mlflow/exceptions.py flags line 136 (until the separate fix lands)
  • Existing clint tests pass: uv run pytest dev/clint/tests/

Implementation Notes

  1. Create rule at dev/clint/src/clint/rules/except_bool_op.py:

    import ast
    
    from clint.rules.base import Rule
    
    
    class ExceptBoolOp(Rule):
        def _message(self) -> str:
            return "Did you mean `except (X, Y):`? Using `or`/`and` in an except handler is a bug."
    
        @staticmethod
        def check(node: ast.ExceptHandler) -> bool:
            return isinstance(node.type, ast.BoolOp)
  2. Register in dev/clint/src/clint/rules/__init__.py:

    • Add from clint.rules.except_bool_op import ExceptBoolOp
    • Add "ExceptBoolOp" to __all__
  3. Add visitor in dev/clint/src/clint/linter.py — add a visit_ExceptHandler method to the Linter class:

    def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None:
        if rules.ExceptBoolOp.check(node):
            self._check(Range.from_node(node), rules.ExceptBoolOp())
    
        self.generic_visit(node)
  4. Add test at dev/clint/tests/rules/test_except_bool_op.py:

    from pathlib import Path
    
    from clint.config import Config
    from clint.linter import Position, Range, lint_file
    from clint.rules import ExceptBoolOp
    
    
    def test_except_bool_op(index_path: Path) -> None:
        code = """
    # Bad - or in except
    try:
        pass
    except ValueError or KeyError:
        pass
    
    # Bad - and in except
    try:
        pass
    except ValueError and KeyError:
        pass
    
    # Bad - chained or
    try:
        pass
    except ValueError or KeyError or TypeError:
        pass
    
    # Good - tuple syntax
    try:
        pass
    except (ValueError, KeyError):
        pass
    
    # Good - single exception
    try:
        pass
    except ValueError:
        pass
    
    # Good - bare except
    try:
        pass
    except:
        pass
    """
        config = Config(select={ExceptBoolOp.name})
        results = lint_file(Path("test.py"), code, config, index_path)
        assert all(isinstance(r.rule, ExceptBoolOp) for r in results)
        assert [r.range for r in results] == [
            Range(Position(4, 0)),
            Range(Position(10, 0)),
            Range(Position(16, 0)),
        ]

PR

Title: Add clint rule to flag except X or Y boolean operator misuse

Body: Add except-bool-op rule to clint that detects except X or Y patterns in except handlers. Due to Python operator precedence, except ValueError or KeyError only catches ValueError — the or evaluates at parse time and silently drops subsequent exception types. The rule suggests using a tuple except (X, Y): instead.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@harupy harupy marked this pull request as ready for review February 17, 2026 01:01
Copilot AI review requested due to automatic review settings February 17, 2026 01:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

…cept handlers

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Copilot AI changed the title [WIP] Add clint rule to flag except X or Y syntax Add clint rule to flag except X or Y boolean operator misuse Feb 17, 2026
Copilot AI requested a review from harupy February 17, 2026 01:07
# Bad - or in except
try:
pass
except ValueError or KeyError:
Copy link
Member

Choose a reason for hiding this comment

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

This is a mistake in 99.9% cases

@github-actions github-actions bot added area/build Build and test infrastructure for MLflow rn/none List under Small Changes in Changelogs. labels Feb 17, 2026

class ExceptBoolOp(Rule):
def _message(self) -> str:
return "Did you mean `except (X, Y):`? Using `or`/`and` in an except handler is a bug."
Copy link
Member

Choose a reason for hiding this comment

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

@copilot

  • Remove backticks around and and or.
  • "is a bug" -> "is likely a mistake"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 0651ff1. Updated the message to remove backticks around or/and and changed "is a bug" to "is likely a mistake".

…ikely a mistake'

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>
Copilot AI requested a review from harupy February 17, 2026 01:27
@harupy harupy enabled auto-merge February 17, 2026 02:43
@harupy harupy added this pull request to the merge queue Feb 17, 2026
Merged via the queue into master with commit 52469a1 Feb 17, 2026
46 of 48 checks passed
@harupy harupy deleted the copilot/add-clint-rule-except-pattern branch February 17, 2026 03:23
@github-actions github-actions bot removed the size/XS label Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build Build and test infrastructure for MLflow rn/none List under Small Changes in Changelogs. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants