[flake8-annotations] Don't suggest NoReturn for functions raising NotImplementedError (ANN201, ANN202, ANN205, ANN206)

[danparizher]

Summary

Fix ANN201, ANN202, ANN205, and ANN206 to not automatically suggest NoReturn/Never return type annotations for functions that raise NotImplementedError. These rules will still flag missing return type annotations but won't provide an auto-fix, allowing developers to manually specify the actual return type that implementations should return.

Fixes #18886

Problem Analysis

The bug occurred because the auto_return_type function in flake8_annotations/helpers.rs detects when all control flow paths in a function raise an exception (Terminal::Raise) and automatically suggests NoReturn/Never as the return type. However, NotImplementedError is semantically different from other exceptions - it's used to indicate abstract methods that should be implemented by subclasses, not methods that truly never return.

When a function raises NotImplementedError, it's typically an abstract method pattern where:

• The base class defines the method signature but doesn't implement it
• Subclasses are expected to override the method with a proper implementation
• The return type should reflect what implementations will actually return, not NoReturn

For example, in Django's serializer pattern:

class BaseSerializer:
    def to_internal_value(self, data):
        raise NotImplementedError('`to_internal_value()` must be implemented.')

This method should have a return type annotation like dict or Any, not NoReturn, because implementations will return actual values.

Approach

The fix modifies the auto_return_type function to:

1. Added semantic analysis capability: Modified auto_return_type to accept a SemanticModel parameter, enabling it to analyze exception types in raise statements.

2. Created detection helper: Implemented only_raises_not_implemented_error() function that:

   • Traverses the function body using a custom visitor to collect all raise statements
   • Checks if all raises are NotImplementedError (or NotImplemented) using semantic analysis
   • Returns true only if all raises are NotImplementedError

3. Modified return type inference: When Terminal::Raise is detected, the function now:

   • First checks if all raises are NotImplementedError
   • If yes, returns None (no auto-fix suggested, but diagnostic still emitted)
   • If no, returns AutoPythonType::Never as before (suggesting NoReturn/Never)

4. Updated all call sites: Updated all 4 call sites in definition.rs to pass checker.semantic():

   • ANN201 (public functions)
   • ANN202 (private functions)
   • ANN205 (static methods)
   • ANN206 (class methods)

The fix ensures that:

• Functions raising NotImplementedError still get flagged for missing return type annotations
• But no auto-fix is suggested, allowing manual specification of the correct return type
• Functions raising other exceptions (like ValueError) still correctly get NoReturn/Never suggestions

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[ntBre]

Could you summarize the similarities/differences from #18927? I quickly skimmed both implementations and this looks similar to the earlier PR. Has @MichaReiser's concern in #18927 (comment) been addressed?

[danparizher]

I missed that, thanks for bringing it back up. I just refactored the implementation:

• Adds a new Terminal::RaiseNotImplemented variant that's detected during the initial Terminal::from_function() traversal
• Eliminates the need for the only_raises_not_implemented_error() function that was doing a second pass
• Updates auto_return_type() to check for Terminal::RaiseNotImplemented directly

The way I did it in the original PR is pretty similar, minus the logic that I just implemented

[ntBre]

Thanks, this looks reasonable to me overall. I just think we can make the SemanticModel a required argument, and we should double check all of the existing uses of Terminal::Raise to be sure they handle Terminal::RaiseNotImplemented correctly.

[ntBre]

Thank you! I had one small question about NotImplemented, but this is otherwise good to go.