astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
30.23k stars 987 forks source link

D102 vs pylint counterpart C0116 (undocumented-public-method) #9149

Open sigma67 opened 8 months ago

sigma67 commented 8 months ago

As noted in https://github.com/astral-sh/ruff/issues/970#issuecomment-1341901539 , in #970 D102 is different from its pylint counterpart C0116.

It only requires that a function has docstring in the class hierarchy and does not require the overriding method to have a docstring (only the base method).

class Foo:
  def validate(self):
    """Doc."""
    ...

class Bar(Foo):
  def validate(self):
    ...

I'm creating a separate issue to give this topic some visibility and to discuss it.

sigma67 commented 8 months ago

The same applies to D105 when overriding a base class' magic method, which already has a docstring. In pylint C0116 this is ok, in D105 it's not.

sigma67 commented 8 months ago

@charliermarsh Perhaps you can comment if this is intended or could be changed? This would not be a breaking change, more of a "softening" of the rule to be more compatible with repositories moving to ruff from pylint.

CC @hmc-cs-mdrissi original report

charliermarsh commented 8 months ago

@sigma67 - The challenge here is that, unlike Pylint, Ruff doesn't do analysis across files right now. So we could support this within a file, but we wouldn't be able to detect that the validate on Bar is an implementation of a parent interface if Foo is defined in another file.

One valid solution here is that we do ignore D102 if you mark the method with typing.override or typing_extensions.override, which signals to type checkers that the method is intended to override a parent method.

sigma67 commented 8 months ago

@charliermarsh I see, thanks for the clarification and the hint regarding typing.override, that's quite helpful.

Are there future plans for cross-file analysis?

charliermarsh commented 8 months ago

@sigma67 -- Yes, it will absolutely be part of Ruff in the future. It's a big project though so will take some time :)