SUGGESTION: enforce docstring consistency in PyRIT with Ruff

[paulinek13]

Related issue: #653

In short, this issue proposes adding Ruff as a dev dependency to enforce documentation consistency in PyRIT.

Why

PyRIT currently lacks automated verification of docs consistency.
The proposed solution will automatically check for the docstring requirements defined in the project's style guide (.github/instructions/style-guide.instructions.md), including:

- Google-style docstring format
- Type information in parameter descriptions
- Documented return values
- Documented exceptions (Raises sections)
- Triple quotes for all docstrings

It will also ensure that, for example, new public classes/methods/functions actually are documented to prevent missing entries in the docs.

Of course, things like that can be manually reviewed during PR reviews, but this could be easily automated + this change makes it now easier in fixing #653, I think.

Proposed Solution

As stated above, the proposal is to add Ruff as a new dev dependency, configure it properly to match as much of the existing style as possible, and add ruff check pyrit/ to the .pre-commit-config.yaml.

https://pypi.org/project/ruff/
https://docs.astral.sh/ruff/

In future ruff could replace flake8/isort/pylint in PyRIT, but for now, I'm proposing to add it just for docs checks.

More info

ruff check pyrit\ takes about 200-600 ms, so it's super fast.

Currently, there are about 1700 "errors":

config I've used:

[tool.ruff.lint.pydocstyle]
convention = "google"

[tool.ruff.lint]
preview = true
select = [
    "D",  # https://docs.astral.sh/ruff/rules/#pydocstyle-d
    "DOC"  # https://docs.astral.sh/ruff/rules/#pydoclint-doc
]
ignore = [
    "D100",  # Missing docstring in public module
    "D200",  # One-line docstring should fit on one line
    "D205",  # 1 blank line required between summary line and description
    "D212",  # Multi-line docstring summary should start at the first line
    "DOC502",  # Raised exception is not explicitly raised: {id}
]
extend-select = [
    "D204",  # 1 blank line required after class docstring
    "D213",  # Multi-line docstring summary should start at the second line
    "D401",  # First line of docstring should be in imperative mood: "{first_line}"
    "D404",  # First word of the docstring should not be "This"
]

[romanlutz]

I'm all for this. It should be part of the pre commit config. We should at the same time evaluate which parts of the existing config are no longer useful. However, fixing 1800 issues is probably going to take a little while. I wonder if we can do it in batches?

[paulinek13]

Thanks for the feedback. We can certainly do this in batches.

I've divided the docstring errors into two main categories:

1. formatting / whitespace / style -- easy to fix in one go across the repo

    118     D202    [*] blank-line-after-function
    105     D213    [*] multi-line-summary-second-line
     44     D411    [*] no-blank-line-before-section
     20     D410    [*] no-blank-line-after-section
      3     D209    [*] new-line-after-last-paragraph
      1     D214    [*] overindented-section
      6     D301    [ ] escape-sequence-in-docstring
      2     D300    [*] triple-single-quotes
      3     D403    [*] first-word-uncapitalized
      3     D416    [*] missing-section-name-colon
     80     D415    [ ] missing-terminal-punctuation

2. public API coverage / completeness / language style -- these will be addressed in separate PRs per module(s) to keep changes manageable

    244     D102    [ ] undocumented-public-method
     71     D107    [ ] undocumented-public-init
     62     D101    [ ] undocumented-public-class
     44     D417    [ ] undocumented-param
     35     D103    [ ] undocumented-public-function
     33     D104    [ ] undocumented-public-package
     27     D105    [ ] undocumented-magic-method
      2     D106    [ ] undocumented-public-nested-class
    191     DOC501  [ ] docstring-missing-exception
    167     DOC201  [ ] docstring-missing-returns
     11     DOC202  [ ] docstring-extraneous-returns
      1     DOC402  [ ] docstring-missing-yields
     21     D404    [ ] docstring-starts-with-this
      8     D418    [ ] overload-with-docstring
    471     D401    [ ] non-imperative-mood

The plan is as follows:

1. I will start with a PR that adds Ruff as a dev dependency along with the initial doc-related config. The same PR will also fix all errors from the first group across the repo (as they are mostly "mechanical" changes and easy to fix in one go).

2. Subsequent PRs would address the second group of errors broken down by modules to keep the PRs manageable. These PRs would need to ensure that all Ruff docstring errors in the targeted module(s) are fixed (ruff check pyrit/module1 pyrit/module2 -> "All checks passed!")

3. Once all the above is done, we can add Ruff to the precommit so that future PRs are blocked if they introduce docs issues.

As for the second point, I think we could track the overall progress of this effort. Maybe a checklist of modules to be done, so (new) contributors can pick/claim one (or more) and work on it.

---

Does this sound like a good approach?