ruff icon indicating copy to clipboard operation
ruff copied to clipboard

[flake8-pyi] Implement PYI057

Open tomasr8 opened this issue 1 year ago • 4 comments

Summary

Adds Y057 from flake8-pyi.

I don't think we can apply any autofix here?

I'm new to rust so apologies in advance if I've made some rookie mistakes :sweat_smile:

Test Plan

cargo test + cargo insta review

tomasr8 avatar May 21 '24 21:05 tomasr8

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+1 -0 violations, +0 -0 fixes in 1 projects; 49 projects unchanged)

python/typeshed (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --output-format concise --preview --select E,F,FA,I,PYI,RUF,UP,W

+ stdlib/_collections_abc.pyi:10:5: PYI057 Do not use `typing.ByteString`, which has unclear semantics and is deprecated
Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
PYI057 1 1 0 0 0

github-actions[bot] avatar May 21 '24 22:05 github-actions[bot]

Hi! The primary reviewer for this is out right now so it might be a bit slower than usual to get reviewed.

Could you change the rule to be in Preview instead of the Stable group?

Additionally, I think use of resolve_qualified_name might be helpful in your implementation:

https://github.com/astral-sh/ruff/blob/f79c980e17bf1ef6064a589dfc5e6b1c288e860f/crates/ruff_python_semantic/src/model.rs#L726-L741

zanieb avatar May 21 '24 22:05 zanieb

Welcome to the project :)

zanieb avatar May 21 '24 22:05 zanieb

Additionally, I think use of resolve_qualified_name might be helpful in your implementation:

I thought there might be something like this already, but couldn't find it. Thanks!

tomasr8 avatar May 21 '24 22:05 tomasr8

oops didn't notice you already fixed the conflict :sweat_smile:

tomasr8 avatar May 29 '24 09:05 tomasr8

hehe, no worries

AlexWaygood avatar May 29 '24 09:05 AlexWaygood

Thanks for the review! (as always :D)

I removed the fix_title implementation and used an enum instead of the full_name string as you suggested.

The bytestring_import function should also be a bit more efficient now since I check for the module id before entering the loop and return early if it doesn't match.

tomasr8 avatar May 29 '24 09:05 tomasr8