TRY004 auto-fix is too aggressive

[WilliamJamieson]

The TRY004 (TC004 in try.ceratops) is a bit too aggressive for me.

It is indicating an error in:

if isinstance(this, some_type):
    # look for a something in `this` and return it, if it is found
   ....

    raise ValueError(f"{this} does not have what your looking for")

This is not a TypeError as what TRY004 is saying it should be, and I don't like the auto-fix trying to correct this. I think this is a ValueError as this does not contain the value I am looking for.

Note that the above also does not follow the example presented in the try.ceratops docs, namely:

if isinstance(my_var, int):
    pass
else:
    raise ValueError(f'{my_var} must be an int')

Should raise a TypeError instead (as it is not the correct type).

It seems like ruff is doing more with TRY004 than checking for errors raised if the type is not what is expected. It seems to be also applying this to cases where a type check is performed and then an error is raised within that check, not if the check fails.

[charliermarsh]

Thanks for filing! Looks like we're not handling the else case faithfully.

What would you suggest as a better methodology for TRY004? Only flag if there are no other statements in the if block, to be more conservative? I think the originating plugin also flags on the case you've described, but definitely correct me if I'm mistaken.

[WilliamJamieson]

I would prefer if it didn't flag if there is a return (method to escape) between the start of the block and the error being raised. That would imply (to me) that the error is not being raised due to the type of something but for some other reason.

[charliermarsh]

That seems reasonable to me.

[WilliamJamieson]

In any case, I don't think this should be autofixed by default.

[charliermarsh]

Let me think on it! We're already working on a more granular API for enabling safe and unsafe fixes of varying degrees. You also have full control to disable fixes for any rule or class of rules like so:

[tool.ruff]
unfixable = ["TRY004", "B"]