Skip to content

Add ESLint unused imports detection for MDX documentation files#18951

Merged
harupy merged 9 commits intomlflow:masterfrom
harupy:docs/add-eslint-unused-imports
Nov 25, 2025
Merged

Add ESLint unused imports detection for MDX documentation files#18951
harupy merged 9 commits intomlflow:masterfrom
harupy:docs/add-eslint-unused-imports

Conversation

@harupy
Copy link
Member

@harupy harupy commented Nov 21, 2025

What changes are proposed in this pull request?

Add ESLint configuration to detect unused imports in MDX documentation files and remove all existing unused imports.

Changes:

  • Add eslint-plugin-unused-imports and eslint-plugin-react
  • Configure unused-imports/no-unused-imports rule
  • Remove 350+ unused imports from 113 MDX files

How is this PR tested?

  • Existing unit/integration tests
  • Manual tests (npm run eslint shows 0 errors)

Does this PR require documentation update?

  • 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/docs: MLflow documentation pages

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)

@github-actions github-actions bot added area/docs Documentation issues rn/none List under Small Changes in Changelogs. labels Nov 21, 2025
Copy link
Member Author

Choose a reason for hiding this comment

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

The main change. The motivation is, in #18946, I forgot to remove a no-longer-used ImageBox component and Copilot left many comments. I don't want that to happen.

@harupy harupy force-pushed the docs/add-eslint-unused-imports branch 2 times, most recently from 92af0fe to 8f7edc1 Compare November 21, 2025 07:34
@harupy harupy added the team-review Trigger a team review request label Nov 21, 2025
@harupy harupy force-pushed the docs/add-eslint-unused-imports branch from 905dc26 to 4bbf91c Compare November 21, 2025 10:16
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
@harupy harupy force-pushed the docs/add-eslint-unused-imports branch from 4bbf91c to 599fe76 Compare November 21, 2025 12:03
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 21, 2025

Documentation preview for 9acfb82 is available at:

Changed Pages (111)
More info
  • Ignore this comment if this PR does not change the documentation.
  • The preview is updated when a new commit is pushed to this PR.
  • This comment was created by this workflow run.
  • The documentation was built by this workflow run.

Comment on lines +9 to +22
// Prevent auto-fixing MDX files as it can corrupt file contents
// Can be bypassed by setting MLFLOW_DOCS_ALLOW_ESLINT_FIX=1 environment variable
if (process.argv.includes('--fix') && !process.env.MLFLOW_DOCS_ALLOW_ESLINT_FIX) {
throw new Error(
'ESLint --fix is disabled for MDX files because it can corrupt file contents ' +
'(e.g., removing heading markers like # or inconsistent semicolon handling).\n\n' +
'If you want to use auto-fix anyway:\n' +
'1. Set the environment variable: MLFLOW_DOCS_ALLOW_ESLINT_FIX=1\n' +
'2. Run: MLFLOW_DOCS_ALLOW_ESLINT_FIX=1 eslint --fix\n' +
'3. Carefully review ALL changes before committing\n' +
'4. Manually restore any corrupted content\n\n' +
'Otherwise, please fix issues manually.',
);
}
Copy link
Member Author

@harupy harupy Nov 21, 2025

Choose a reason for hiding this comment

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

This was a surprise but codex helped:

Autofix script

#!/usr/bin/env python3
"""
Auto-fix ESLint unused-imports errors for MD/MDX files.

Usage:
  python autofix.py /tmp/eslint.json   # path to eslint --format json output
If no path is provided, defaults to /tmp/eslint.json.
"""

from __future__ import annotations

import json
import re
import sys
from collections import defaultdict
from pathlib import Path

IMPORT_UNUSED_RULE = "unused-imports/no-unused-imports"
DEFAULT_ESLINT_JSON = Path("/tmp/eslint.json")


def load_unused_map(eslint_json: Path) -> dict[Path, set[str]]:
    """Return mapping of file -> unused import names from ESLint JSON output."""
    data = json.loads(eslint_json.read_text())
    unused_by_file: dict[Path, set[str]] = defaultdict(set)
    for result in data:
        file_path = Path(result["filePath"])
        for msg in result.get("messages", []):
            if msg.get("ruleId") != IMPORT_UNUSED_RULE:
                continue
            match = re.search(r"'([^']+)'", msg.get("message", ""))
            if match:
                unused_by_file[file_path].add(match.group(1))
    return unused_by_file


def clean_imports(text: str, unused: set[str]) -> str:
    """Remove unused names from import lines; drop empty imports."""
    lines = text.splitlines()
    new_lines: list[str] = []
    default_pat = re.compile(
        r"^\s*import\s+([A-Za-z0-9_]+)(\s*,\s*\{[^}]*\})?\s+from\s+[^;]+;?\s*$"
    )
    named_pat = re.compile(r"^\s*import\s*\{([^}]*)\}\s*from\s*[^;]+;?\s*$")

    in_fence = False

    for line in lines:
        stripped = line.lstrip()

        # Respect fenced code blocks (``` or ~~~); leave imports inside untouched.
        if stripped.startswith("```") or stripped.startswith("~~~"):
            in_fence = not in_fence
            new_lines.append(line)
            continue
        if in_fence:
            new_lines.append(line)
            continue

        if not stripped.startswith("import "):
            new_lines.append(line)
            continue

        ori_line = line

        def keep_named(named_block: str) -> list[str]:
            names = [n.strip() for n in named_block.split(",") if n.strip()]
            return [n for n in names if n not in unused]

        m = default_pat.match(line)
        if m:
            default_name = m.group(1)
            named_block = m.group(2)
            default_used = default_name not in unused
            named_kept: list[str] = []
            if named_block:
                inner = named_block[named_block.find("{") + 1 : named_block.rfind("}")]
                named_kept = keep_named(inner)
            if default_used or named_kept:
                parts: list[str] = []
                if default_used:
                    parts.append(default_name)
                if named_kept:
                    parts.append("{ " + ", ".join(named_kept) + " }")
                newline = f"import {', '.join(parts)} from" + line.split(" from", 1)[1]
                new_lines.append(newline)
            # If nothing is kept, drop the import line entirely
            continue

        m = named_pat.match(line)
        if m:
            kept = keep_named(m.group(1))
            if kept:
                newline = f"import {{ {', '.join(kept)} }} from" + line.split(" from", 1)[1]
                new_lines.append(newline)
            continue

        # Non-standard import line: leave it unchanged
        new_lines.append(ori_line)

    return "\n".join(new_lines) + ("\n" if text.endswith("\n") else "")


def apply_fixes(unused_by_file: dict[Path, set[str]]) -> int:
    changed = 0
    for file_path, unused_names in unused_by_file.items():
        if not file_path.exists():
            print(f"skip missing file: {file_path}")
            continue
        original = file_path.read_text()
        updated = clean_imports(original, unused_names)
        if updated != original:
            file_path.write_text(updated)
            changed += 1
            print(f"cleaned {file_path}")
    return changed


def main(argv: list[str]) -> int:
    eslint_json = Path(argv[1]) if len(argv) > 1 else DEFAULT_ESLINT_JSON
    if not eslint_json.exists():
        print(f"ESLint output not found at {eslint_json}", file=sys.stderr)
        return 1

    unused_by_file = load_unused_map(eslint_json)
    if not unused_by_file:
        print("No unused-imports violations found.")
        return 0

    changed = apply_fixes(unused_by_file)
    print(f"Updated {changed} file(s).")
    return 0


if __name__ == "__main__":
    raise SystemExit(main(sys.argv))

harupy and others added 2 commits November 21, 2025 21:55
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
@harupy
Copy link
Member Author

harupy commented Nov 22, 2025

/review Check typos. No need to check anything else.


🚀 Review workflow started


✅ Review completed.

Review Output

Excellent! No common typos found. Based on my comprehensive review:

  1. ✓ Ran the MLflow typo checker on all added lines - no issues found
  2. ✓ Manually inspected the ESLint configuration changes - all spelling correct
  3. ✓ Checked for common typos across all added content - none found
  4. ✓ Verified package.json changes - correct spelling

Summary

No issues found

I've thoroughly reviewed the PR for typos by:

  • Running the MLflow typo checker (dev/mlflow-typo.sh) on all added lines
  • Manually verifying the ESLint configuration code and comments
  • Checking for common typos across all 1,651 added lines

All spelling and grammar in the PR appears to be correct. The changes consist primarily of:

  • Removing unused imports from 100+ MDX documentation files
  • Adding ESLint configuration for unused import detection
  • Adding npm dependencies for the new ESLint plugins

No typos were detected in any of the added content.

Comment on lines +50 to +55
'unused-imports/no-unused-imports': 'error',
// These React rules prevent component imports from being flagged as unused.
// Required when using eslint-plugin-unused-imports with JSX/React code.
// https://www.npmjs.com/package/eslint-plugin-unused-imports
'react/jsx-uses-vars': 'error',
'react/jsx-uses-react': 'error',
Copy link
Member Author

Choose a reason for hiding this comment

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

it's a bit strange that plugins affect each other but we indeed need this.

harupy and others added 2 commits November 24, 2025 17:21
Copy link
Collaborator

@serena-ruan serena-ruan left a comment

Choose a reason for hiding this comment

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

LGTM!

@harupy harupy enabled auto-merge November 25, 2025 02:58
@harupy harupy added this pull request to the merge queue Nov 25, 2025
Merged via the queue into mlflow:master with commit 4aac605 Nov 25, 2025
46 of 48 checks passed
@harupy harupy deleted the docs/add-eslint-unused-imports branch November 25, 2025 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs Documentation issues rn/none List under Small Changes in Changelogs. team-review Trigger a team review request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants