Repository: mlflow/mlflow
Pull request: Auto-generate ID for Rule Classes in clint using `__init_subclass__` with itertools.count
<details><summary>&#x1F6E0 DevTools &#x1F6E0</summary>
<p>

[![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/Copilot/mlflow/pull/16307?quickstart=1)

#### Install mlflow from this PR

```
# mlflow
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/16307/merge
# mlflow-skinny
pip install git+https://github.com/mlflow/mlflow.git@refs/pull/16307/merge#subdirectory=skinny
```

For Databricks, use the following command:

```
%sh curl -LsSf https://raw.githubusercontent.com/mlflow/mlflow/HEAD/dev/install-skinny.sh | sh -s pull/16307/merge
```

</p>
</details>

This PR implements automatic ID generation for Rule classes in the clint linter, eliminating the need to manually implement `_id()` methods in each Rule subclass.

## Problem

Previously, each Rule subclass had to manually implement the `_id()` method to return a unique identifier (e.g., "MLF0001", "MLF0002", etc.). This was error-prone and required manual tracking of which IDs had been used across 25+ rule classes.

## Solution

Implemented automatic ID generation using `__init_subclass__` with `itertools.count`:

- **Auto-generation mechanism**: Uses `__init_subclass__` to assign unique IDs when classes are defined
- **Thread-safe incrementing**: Uses `itertools.count` for reliable auto-incrementing
- **Preserves existing IDs**: All 25 existing rule IDs (MLF0001-MLF0025) remain exactly the same
- **Clean implementation**: Removes 79 lines of boilerplate code while adding only 12 lines

## Key Changes

1. **Updated Rule base class** in `dev/clint/src/clint/rules.py`:
   ```python
   class Rule(ABC):
       _id_counter = itertools.count(start=1)
       _generated_id: str

       def __init_subclass__(cls, **kwargs):
           super().__init_subclass__(**kwargs)
           # Only generate ID for concrete classes
           if not inspect.isabstract(cls):
               cls._generated_id = f"MLF{next(cls._id_counter):04d}"

       def _id(self) -> str:
           """Return the auto-generated ID for this rule."""
           return self._generated_id
   ```

2. **Removed manual `_id()` implementations** from all 25 rule subclasses while preserving their exact functionality.

## Benefits

- ✅ **Eliminates manual ID tracking** - No more need to remember which IDs are taken
- ✅ **Reduces boilerplate code** - 67 lines net reduction (79 deleted, 12 added)  
- ✅ **Prevents ID collisions** - Automatic incrementing ensures uniqueness
- ✅ **Makes adding new rules easier** - New rules automatically get the next available ID
- ✅ **Backward compatible** - All existing IDs preserved exactly
- ✅ **Thread-safe** - Uses `itertools.count` for reliable auto-incrementing

## Verification

All 25 rule IDs are verified to match their original values:
- NoRst: MLF0001 ✓
- LazyBuiltinImport: MLF0002 ✓  
- MlflowClassName: MLF0003 ✓
- ... (all 25 rules verified)

The clint linter continues to work correctly with the new auto-generation mechanism.

Fixes #16306.

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.

Problem statement:
Auto-generate ID for Rule Classes in clint
# Auto-generate ID for Rule Classes

_This plan was created by Claude to document the implementation approach for auto-generating Rule IDs._

## Problem Statement

Currently, each Rule subclass in `dev/clint/src/clint/rules.py` must manually implement the `_id()` method to return a unique identifier (e.g., "MLF0001", "MLF0002", etc.). This is error-prone and requires manual tracking of which IDs have been used.

## Goal

Implement automatic ID generation that assigns a unique ID to each Rule subclass when it's defined, eliminating the need to manually implement `_id()` in each subclass.

## Proposed Solution: Using `__init_subclass__` with `itertools.count`

### Implementation

```python
import itertools
from abc import ABC
import inspect

class Rule(ABC):
    _id_counter = itertools.count(start=1)
    _generated_id: str

    def __init_subclass__(cls, **kwargs):
        super().__init_subclass__(**kwargs)
        # Only generate ID for concrete classes
        if not inspect.isabstract(cls):
            cls._generated_id = f"MLF{next(cls._id_counter):04d}"

    def _id(self) -> str:
        """Return the auto-generated ID for this rule."""
        return self._generated_id

    # Remove @abstractmethod decorator from _id
    # The rest of the Rule class remains unchanged...
```

### Key Features

- Uses `itertools.count` for thread-safe, automatic incrementing
- IDs are assigned when classes are defined (import time)
- Abstract classes are skipped
- Simple and clean implementation

## Implementation Tasks

1. Update the `Rule` base class in `dev/clint/src/clint/rules.py` with the new implementation
2. Remove `_id()` methods from all 25 existing rule classes
3. Ensure class definition order is preserved to maintain existing ID assignments
4. Update tests if necessary

## Benefits

- Eliminates manual ID tracking
- Reduces boilerplate code
- Prevents ID collisions
- Makes adding new rules easier

Hints from issue discussion:

Commit message: Implement auto-generation of Rule IDs using __init_subclass__ with itertools.count

Co-authored-by: harupy <17039389+harupy@users.noreply.github.com>

Patch to review:
diff --git a/dev/clint/src/clint/rules.py b/dev/clint/src/clint/rules.py
index c8ed92d46fd28..825d840e2f689 100644
--- a/dev/clint/src/clint/rules.py
+++ b/dev/clint/src/clint/rules.py
@@ -1,6 +1,8 @@
 from __future__ import annotations
 
 import ast
+import inspect
+import itertools
 import re
 from abc import ABC, abstractmethod
 
@@ -11,12 +13,18 @@
 
 class Rule(ABC):
     _CLASS_NAME_TO_RULE_NAME_REGEX = re.compile(r"(?<!^)(?=[A-Z])")
+    _id_counter = itertools.count(start=1)
+    _generated_id: str
+
+    def __init_subclass__(cls, **kwargs):
+        super().__init_subclass__(**kwargs)
+        # Only generate ID for concrete classes
+        if not inspect.isabstract(cls):
+            cls._generated_id = f"MLF{next(cls._id_counter):04d}"
 
-    @abstractmethod
     def _id(self) -> str:
-        """
-        Return a unique identifier for this rule.
-        """
+        """Return the auto-generated ID for this rule."""
+        return self._generated_id
 
     @property
     def id(self) -> str:
@@ -41,49 +49,31 @@ def name(self) -> str:
 
 
 class NoRst(Rule):
-    def _id(self) -> str:
-        return "MLF0001"
-
     def _message(self) -> str:
         return "Do not use RST style. Use Google style instead."
 
 
 class LazyBuiltinImport(Rule):
-    def _id(self) -> str:
-        return "MLF0002"
-
     def _message(self) -> str:
         return "Builtin modules must be imported at the top level."
 
 
 class MlflowClassName(Rule):
-    def _id(self) -> str:
-        return "MLF0003"
-
     def _message(self) -> str:
         return "Should use `Mlflow` in class name, not `MLflow` or `MLFlow`."
 
 
 class TestNameTypo(Rule):
-    def _id(self) -> str:
-        return "MLF0004"
-
     def _message(self) -> str:
         return "This function looks like a test, but its name does not start with 'test_'."
 
 
 class LogModelArtifactPath(Rule):
-    def _id(self) -> str:
-        return "MLF0005"
-
     def _message(self) -> str:
         return "`artifact_path` parameter of `log_model` is deprecated. Use `name` instead."
 
 
 class ExampleSyntaxError(Rule):
-    def _id(self) -> str:
-        return "MLF0006"
-
     def _message(self) -> str:
         return "This example has a syntax error."
 
@@ -92,9 +82,6 @@ class MissingDocstringParam(Rule):
     def __init__(self, params: set[str]) -> None:
         self.params = params
 
-    def _id(self) -> str:
-        return "MLF0007"
-
     def _message(self) -> str:
         return f"Missing parameters in docstring: {self.params}"
 
@@ -103,9 +90,6 @@ class ExtraneousDocstringParam(Rule):
     def __init__(self, params: set[str]) -> None:
         self.params = params
 
-    def _id(self) -> str:
-        return "MLF0008"
-
     def _message(self) -> str:
         return f"Extraneous parameters in docstring: {self.params}"
 
@@ -114,17 +98,11 @@ class DocstringParamOrder(Rule):
     def __init__(self, params: list[str]) -> None:
         self.params = params
 
-    def _id(self) -> str:
-        return "MLF0009"
-
     def _message(self) -> str:
         return f"Unordered parameters in docstring: {self.params}"
 
 
 class ImplicitOptional(Rule):
-    def _id(self) -> str:
-        return "MLF0010"
-
     def _message(self) -> str:
         return "Use `Optional` if default value is `None`"
 
@@ -170,17 +148,11 @@ def _is_none(value: ast.AnnAssign) -> bool:
 
 
 class OsEnvironSetInTest(Rule):
-    def _id(self) -> str:
-        return "MLF0011"
-
     def _message(self) -> str:
         return "Do not set `os.environ` in test directly. Use `monkeypatch.setenv` (https://docs.pytest.org/en/stable/reference/reference.html#pytest.MonkeyPatch.setenv)."
 
 
 class OsEnvironDeleteInTest(Rule):
-    def _id(self) -> str:
-        return "MLF0012"
-
     def _message(self) -> str:
         return "Do not delete `os.environ` in test directly. Use `monkeypatch.delenv` (https://docs.pytest.org/en/stable/reference/reference.html#pytest.MonkeyPatch.delenv)."
 
@@ -189,9 +161,6 @@ class ForbiddenTopLevelImport(Rule):
     def __init__(self, module: str) -> None:
         self.module = module
 
-    def _id(self) -> str:
-        return "MLF0013"
-
     def _message(self) -> str:
         return (
             f"Importing module `{self.module}` at the top level is not allowed "
@@ -200,9 +169,6 @@ def _message(self) -> str:
 
 
 class UseSysExecutable(Rule):
-    def _id(self) -> str:
-        return "MLF0014"
-
     def _message(self) -> str:
         return (
             "Use `[sys.executable, '-m', 'mlflow', ...]` when running mlflow CLI in a subprocess."
@@ -246,9 +212,6 @@ def _is_abstract_method(node: ast.FunctionDef | ast.AsyncFunctionDef) -> bool:
 
 
 class InvalidAbstractMethod(Rule):
-    def _id(self) -> str:
-        return "MLF0015"
-
     def _message(self) -> str:
         return (
             "Abstract method should only contain a single statement/expression, "
@@ -288,9 +251,6 @@ class IncorrectTypeAnnotation(Rule):
     def __init__(self, type_hint: str) -> None:
         self.type_hint = type_hint
 
-    def _id(self) -> str:
-        return "MLF0016"
-
     @staticmethod
     def check(node: ast.Name) -> bool:
         return node.id in IncorrectTypeAnnotation.MAPPING
@@ -309,9 +269,6 @@ def __init__(self, *, full_name: str, allowlist: list[str]) -> None:
         self.full_name = full_name
         self.allowlist = allowlist
 
-    def _id(self) -> str:
-        return "MLF0017"
-
     def _message(self) -> str:
         return (
             f"`{self.full_name}` is not allowed to use. Only {self.allowlist} are allowed. "
@@ -322,9 +279,6 @@ def _message(self) -> str:
 
 
 class MarkdownLink(Rule):
-    def _id(self) -> str:
-        return "MLF0018"
-
     def _message(self) -> str:
         return (
             "Markdown link is not supported in docstring. "
@@ -333,25 +287,16 @@ def _message(self) -> str:
 
 
 class LazyModule(Rule):
-    def _id(self) -> str:
-        return "MLF0019"
-
     def _message(self) -> str:
         return "Module loaded by `LazyLoader` must be imported in `TYPE_CHECKING` block."
 
 
 class EmptyNotebookCell(Rule):
-    def _id(self) -> str:
-        return "MLF0020"
-
     def _message(self) -> str:
         return "Empty notebook cell. Remove it or add some content."
 
 
 class ForbiddenSetActiveModelUsage(Rule):
-    def _id(self) -> str:
-        return "MLF0021"
-
     def _message(self) -> str:
         return (
             "Usage of `set_active_model` is not allowed in mlflow, use `_set_active_model` instead."
@@ -369,9 +314,6 @@ def check(node: ast.Call, resolver: Resolver) -> bool:
 
 
 class ForbiddenTraceUIInNotebook(Rule):
-    def _id(self) -> str:
-        return "MLF0022"
-
     def _message(self) -> str:
         return (
             "Found the MLflow Trace UI iframe in the notebook. "
@@ -382,9 +324,6 @@ def _message(self) -> str:
 
 
 class PytestMarkRepeat(Rule):
-    def _id(self) -> str:
-        return "MLF0023"
-
     def _message(self) -> str:
         return (
             "@pytest.mark.repeat decorator should not be committed. "
@@ -411,9 +350,6 @@ def _is_valid_version(version: str) -> bool:
 
 
 class UnnamedThread(Rule):
-    def _id(self) -> str:
-        return "MLF0024"
-
     def _message(self) -> str:
         return (
             "`threading.Thread()` must be called with a `name` argument to improve debugging "
@@ -433,9 +369,6 @@ def check(node: ast.Call, resolver: Resolver) -> bool:
 
 
 class InvalidExperimentalDecorator(Rule):
-    def _id(self) -> str:
-        return "MLF0025"
-
     def _message(self) -> str:
         return (
             "Invalid usage of `@experimental` decorator. It must be used with a `version` "


Code review comments:
diff_hunk: @@ -11,12 +13,18 @@
 
 class Rule(ABC):
     _CLASS_NAME_TO_RULE_NAME_REGEX = re.compile(r"(?<!^)(?=[A-Z])")
+    _id_counter = itertools.count(start=1)
+    _generated_id: str
+
+    def __init_subclass__(cls, **kwargs):
+        super().__init_subclass__(**kwargs)
+        # Only generate ID for concrete classes
+        if not inspect.isabstract(cls):
+            cls._generated_id = f"MLF{next(cls._id_counter):04d}"
 
-    @abstractmethod
     def _id(self) -> str:
path: dev/clint/src/clint/rules.py
text: @user1:
let's remove this method and make the `id` property return `self._generated_id`.

@author:
Done! Removed the `_id()` method and made the `id` property return `self._generated_id` directly. All Rule IDs remain the same (MLF0001-MLF0025). Commit: 2405c3c