Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Highlighting for unhandled exceptions from docstrings #12628

Open
divaltor opened this issue Aug 2, 2024 · 3 comments
Open

Highlighting for unhandled exceptions from docstrings #12628

divaltor opened this issue Aug 2, 2024 · 3 comments
Labels
docstring Related to docstring linting or formatting type-inference Requires more advanced type inference.

Comments

@divaltor
Copy link

divaltor commented Aug 2, 2024

Thanks for your work and the awesome tool ❤️

Problem

Some docstring formats support raise blocks for describing potential exceptions a function may throw. However, neither LSP (pyright) nor IDEs (PyCharm) can suggest handling these exceptions when a function is called. This limitation often leads to two bad practices:

  • Handling all types of errors using a broad except Exception block, which can mask specific issues and make debugging more difficult.
  • Completely overlooking potential exceptions, resulting in unhandled errors that can cause unexpected runtime bugs.

Is it possible to create a rule for highlighting unhandled exceptions? Most related feature in PHP Storm - https://blog.jetbrains.com/phpstorm/2017/11/bring-exceptions-under-control/#unhandled-exception

def positive_sum(a: int, b: int) -> int:
	"""
	Summarize two positive integers

	Raises:
		ValueError: when value is not positive
	"""
	if a < 0 or b < 0:
		raise ValueError('value must be positive integer')

	return a + b

positive_sum(15, 15)   <-- RUF999: Unhandled ValueError exception

try:
	positive_sum(15, 15)  <-- OK
except ValueError:
	...

try:
	positive_sum(15, 15)  <-- OK, but may be should consider case when user use common `Exception` for handle it
except Exception:
	...

But there can be problem (may be, I didn't read a whole ruff code to understand how parser and semantic work):

def validate_number(num: int) -> int:
	"""
	Validate number and return error if it's not positive

	Raises:
		ValueError: when value is not positive
	"""
	if num < 0:
		raise ValueError('value must be positive integer')
	
	return num

# There is no docstring
def positive_sum(a: int, b: int) -> int:
	validate_number(a)  <-- RUF999: Unhandled ValueError exception
	validate_number(b)  <-- RUF999: Unhandled ValueError exception

	return a + b

positive_sum(15, 15)  <-- RUF999: Unhandled ValueError exception

Also there could be cases when we use assert statements without docstring, like

def positive_sum(a: int, b: int) -> int:
	assert a > 0
	assert b > 0

	return a + b

# Strongly not sure if should cover such case, because assertion is supposed to find bugs by `fuzzing`
positive_sum(15, 15)  <-- RUF999: Unhandled AssertionError exception

Is it possible to implement such a rule at all? Because my concern is that is job for LSP, not for linter.

Possible limitations

  1. Ruff can't analyze external libraries or their docstrings. This means it's not possible to highlight code that calls external libraries, like requests.
  2. Developers may forget to update docstrings after modifying code, leading to outdated or inaccurate exception information.
@MichaReiser
Copy link
Member

Thanks for the kind words.

If I'm not mistaken than this rule shipped as part of ruff 0.5.5 but under preview mode only. See #11471

@divaltor
Copy link
Author

divaltor commented Aug 2, 2024

Thanks for the kind words.

If I'm not mistaken than this rule shipped as part of ruff 0.5.5 but under preview mode only. See #11471

I guess this rule just prevent cases when user forgets to update its docstring, but do nothing when user actually executes function with Raises block, see my example again (or look at PHPStorm blog which I've attached to at the top)

@MichaReiser
Copy link
Member

Oh yeah sorry. That's something else. I don't think we have the necessary infrastructure in place today because ruff can't do any cross-file analysis.

@AlexWaygood AlexWaygood added the docstring Related to docstring linting or formatting label Aug 2, 2024
@MichaReiser MichaReiser added type-inference Requires more advanced type inference. and removed multifile-analysis labels Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docstring Related to docstring linting or formatting type-inference Requires more advanced type inference.
Projects
None yet
Development

No branches or pull requests

3 participants