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

Derive Hash instead of implementing it by hand #1890

Merged
merged 1 commit into from
Jan 16, 2023

Conversation

not-my-profile
Copy link
Contributor

@not-my-profile not-my-profile commented Jan 15, 2023

No description provided.

@not-my-profile not-my-profile marked this pull request as draft January 15, 2023 08:40
@not-my-profile
Copy link
Contributor Author

Marked this as a draft since #1891 should be merged first.

src/settings/mod.rs Outdated Show resolved Hide resolved
The caching mechanism of the CLI (ruff_cli::cache) relies on
ruff::settings::Settings implementing the Hash trait.

The ruff::settings::Settings struct previously couldn't automatically
derive the Hash implementation via the #[derive(Hash)] macro attribute
since some of its field types intentionally[1][2] don't implement Hash
(namely regex::Regex, globset::GlobMatcher and globset::GlobSet and
HashMap and HashSet from the standard library).

The code therefore previously implemented the Hash trait by hand for the
whole struct. Implementing Hash by hand for structs that are subject to
change is a bad idea since it's very easy to forget to update the Hash
implementation when adding a new field to the struct. And the Hash
implementation indeed was already incorrect by omitting several fields
from the hash.

This commit introduces wrapper types for Regex, GlobMatcher, GlobSet,
HashSet & HashMap that implement Hash so that we can still add
#[derive(Hash)] to the Settings struct, guaranteeing a correct hash
implementation.

[1]: rust-lang/regex#364 (comment)
[2]: The standard library doesn't impl<T: Hash + Ord> Hash for HashSet<T>
     presumably since sorted() requires an allocation and Hash
     implementations are generally expected to work without allocations.
@not-my-profile not-my-profile marked this pull request as ready for review January 16, 2023 04:43
@charliermarsh charliermarsh merged commit 8c18b28 into astral-sh:main Jan 16, 2023
renovate bot added a commit to ixm-one/pytest-cmake-presets that referenced this pull request Jan 17, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [ruff](https://github.com/charliermarsh/ruff) | `^0.0.223` ->
`^0.0.224` |
[![age](https://badges.renovateapi.com/packages/pypi/ruff/0.0.224/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/pypi/ruff/0.0.224/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/pypi/ruff/0.0.224/compatibility-slim/0.0.223)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/pypi/ruff/0.0.224/confidence-slim/0.0.223)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>charliermarsh/ruff</summary>

###
[`v0.0.224`](https://github.com/charliermarsh/ruff/releases/tag/v0.0.224)

[Compare
Source](https://github.com/charliermarsh/ruff/compare/v0.0.223...v0.0.224)

#### What's Changed

- Re-run benchmark and update documentation by
[@&#8203;charliermarsh](https://github.com/charliermarsh) in
[astral-sh/ruff#1907
- Derive Hash instead of implementing it by hand by
[@&#8203;not-my-profile](https://github.com/not-my-profile) in
[astral-sh/ruff#1890
- Add backticks to B904's message by
[@&#8203;harupy](https://github.com/harupy) in
[astral-sh/ruff#1914
- Refactor `flake8_tidy_imports` by
[@&#8203;not-my-profile](https://github.com/not-my-profile) in
[astral-sh/ruff#1909
- Trigger update to pre-commit mirror after pypi publish by
[@&#8203;pmbarrett314](https://github.com/pmbarrett314) in
[astral-sh/ruff#1910
- Rewrite `lru_cache` to `cache` on Python 3.9+ by
[@&#8203;charliermarsh](https://github.com/charliermarsh) in
[astral-sh/ruff#1918
- Avoid syntax errors when fixing parenthesized unused variables by
[@&#8203;charliermarsh](https://github.com/charliermarsh) in
[astral-sh/ruff#1919
- Add some new testimonials by
[@&#8203;charliermarsh](https://github.com/charliermarsh) in
[astral-sh/ruff#1921
- Avoid removing statements that contain side-effects by
[@&#8203;charliermarsh](https://github.com/charliermarsh) in
[astral-sh/ruff#1920
- Add benchmark scripts for no-IO by
[@&#8203;charliermarsh](https://github.com/charliermarsh) in
[astral-sh/ruff#1925
- Add flake8-pie PIE796: prefer-unique-enum by
[@&#8203;ljesparis](https://github.com/ljesparis) in
[astral-sh/ruff#1923
- \[pyupgrade] Automatically rewrite format-strings to f-strings by
[@&#8203;colin99d](https://github.com/colin99d) in
[astral-sh/ruff#1905

#### New Contributors

- [@&#8203;pmbarrett314](https://github.com/pmbarrett314) made their
first contribution in
[astral-sh/ruff#1910
- [@&#8203;ljesparis](https://github.com/ljesparis) made their first
contribution in
[astral-sh/ruff#1923

**Full Changelog**:
astral-sh/ruff@v0.0.223...v0.0.224

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/ixm-one/pytest-cmake-presets).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDIuNyIsInVwZGF0ZWRJblZlciI6IjM0LjEwMi43In0=-->

Signed-off-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants