Skip to content

Conversation

@mortezaPRK
Copy link
Contributor

@mortezaPRK mortezaPRK commented Jan 27, 2025

Fix generating lookup for repeated.items.int32.in

This PR fixes the in field validation for repeated int32 fields.

The current generated code doesn't emit the lookup map e.g.

// message DoSomethingRequest {
//   repeated int32 field_with_int32_type = 4 [ (validate.rules).repeated.items.int32 = { in: [25,50] } ];
// }

// Generated code doesn't have this section
var _DoSomethingRequest_FieldWithInt32Type_InLookup = map[int32]struct{}{
	25:  {},
	50:  {},
}

I also added a Test cases for both int32 and int64

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2025

CLA assistant check
All committers have signed the CLA.

@mortezaPRK mortezaPRK marked this pull request as ready for review January 27, 2025 16:32
@rodaine
Copy link
Member

rodaine commented Jan 27, 2025

Hi @mortezaPRK thanks for the patch! Looks like the lookup was also not generated in the C++ codegen either.

@mortezaPRK
Copy link
Contributor Author

Hey @rodaine

Unfortunately I can't run the tests locally (I tried with Docker and it doesn't work), and I have no idea how C++ code looks like.

Should I remove the test cases I added? I found this PR which is doing something similar tho.

@nicksnyder
Copy link
Member

@mortezaPRK maybe the best path forward is to split this into 2 different PRs. Keep the fixes for Go in this PR and remove the tests you added so we can get this merged as an incremental improvement. Then open a separate PR to add the tests (which will still fail, but will be a reminder that there is a gap in C++ codegen).

@mortezaPRK
Copy link
Contributor Author

mortezaPRK commented Jan 29, 2025

@nicksnyder Agree. I updated the PR.

I'll create another PR for the test cases

@rodaine rodaine changed the title Fix in rule for repeatet int32 Fix in rule for repeated int32 Jan 29, 2025
Copy link
Member

@rodaine rodaine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks again

@rodaine rodaine merged commit 94f2b56 into bufbuild:main Jan 29, 2025
5 checks passed
rodaine added a commit that referenced this pull request Feb 3, 2025
Second part of the #1240 

This PR adds support for `in` rule for `repeated int32/int64` fields for
C

Updated the testcases

Co-authored-by: Chris Roche <[email protected]>
rodaine pushed a commit that referenced this pull request Sep 8, 2025
Bumps the python-root group with 1 update:
[twine](https://github.com/pypa/twine).

Updates `twine` from 6.1.0 to 6.2.0
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/pypa/twine/blob/main/docs/changelog.rst">twine's
changelog</a>.</em></p>
<blockquote>
<h2>twine 6.2.0 (2025-09-04)</h2>
<p>Features
^^^^^^^^</p>
<ul>
<li>
<p>Automatically refresh short-lived PyPI token in long running Trusted
Publishing uploads.</p>
<p>In the event that a trusted publishing upload job is taking longer
than the
validity period of a trusted publishing token (15 minutes at the time of
this
writing), <em>and</em> we are already 10 minutes into that validity
period, we will
begin to attempt to replace the token on each subsequent request.
(<code>[#1246](pypa/twine#1246)
&lt;https://github.com/pypa/twine/issues/1246&gt;</code>_)</p>
</li>
</ul>
<p>Bugfixes
^^^^^^^^</p>
<ul>
<li>Fix compatibility kludge for invalid License-File metadata entries
emitted by
build backends to work also with <code>packaging</code> version 24.0.
(<code>[#1217](pypa/twine#1217)
&lt;https://github.com/pypa/twine/issues/1217&gt;</code>_)</li>
<li>Fix a couple of incorrectly rendered error messages.
(<code>[#1224](pypa/twine#1224)
&lt;https://github.com/pypa/twine/issues/1224&gt;</code>_)</li>
<li><code>twine</code> now enforces <code>keyring &gt;= 21.2.0</code>,
which was previously
implicitly required by API usage.
(<code>[#1229](pypa/twine#1229)
&lt;https://github.com/pypa/twine/issues/1229&gt;</code>_)</li>
<li><code>twine</code> now catches <code>configparser.Error</code> to
prevent accidental
leaks of secret tokens or passwords to the user's console.
(<code>[#1240](pypa/twine#1240)
&lt;https://github.com/pypa/twine/issues/1240&gt;</code>_)</li>
</ul>
<p>Deprecations and Removals
^^^^^^^^^^^^^^^^^^^^^^^^^</p>
<ul>
<li>
<p>Remove hacks that support <code>--skip-existing</code> for indexes
other than PyPI and
TestPyPI.</p>
<p>To date, these hacks continue to accrue and there have been numerous
issues
with them, not the least of which being that every time we update them,
the
paid index providers change things to break the compatibility we
implement
for them. Beyond that, these hacks do not work when text is
internationalized
in the response from the index provider.</p>
<p>For a sample of past issues, see:</p>
<ul>
<li>
<p><a
href="https://redirect.github.com/pypa/twine/issues/1251">pypa/twine#1251</a></p>
</li>
<li>
<p><a
href="https://redirect.github.com/pypa/twine/issues/918">pypa/twine#918</a></p>
</li>
<li>
<p><a
href="https://redirect.github.com/pypa/twine/issues/856">pypa/twine#856</a></p>
</li>
<li>
<p><a
href="https://redirect.github.com/pypa/twine/issues/693">pypa/twine#693</a></p>
</li>
<li>
<p><a
href="https://redirect.github.com/pypa/twine/issues/332">pypa/twine#332</a>
(<code>[#1251](pypa/twine#1251)
&lt;https://github.com/pypa/twine/issues/1251&gt;</code>_)</p>
</li>
</ul>
</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/pypa/twine/commit/14ceb29585a54f679e415b351aece2acc19aa4b2"><code>14ceb29</code></a>
Update changelog for 6.2.0 (<a
href="https://redirect.github.com/pypa/twine/issues/1264">#1264</a>)</li>
<li><a
href="https://github.com/pypa/twine/commit/60e377bd5b340b90504c9e4e976eae926d9c9ddc"><code>60e377b</code></a>
build(deps): bump actions/checkout from 4.2.2 to 5.0.0 (<a
href="https://redirect.github.com/pypa/twine/issues/1263">#1263</a>)</li>
<li><a
href="https://github.com/pypa/twine/commit/88821f278022011496c0ccb0d8ae40ab9d6c5c3d"><code>88821f2</code></a>
feat(package): remove MD5 hashing entirely (<a
href="https://redirect.github.com/pypa/twine/issues/1262">#1262</a>)</li>
<li><a
href="https://github.com/pypa/twine/commit/ce5fe530511c13816e51dbbd13892f10274a87b1"><code>ce5fe53</code></a>
build(deps): bump actions/download-artifact from 4.3.0 to 5.0.0</li>
<li><a
href="https://github.com/pypa/twine/commit/6a696edefec47e842d11de468caf6c7e69694976"><code>6a696ed</code></a>
PEP 639 compliance</li>
<li><a
href="https://github.com/pypa/twine/commit/91753343daa1366711a441b4a13bd1b31146909b"><code>9175334</code></a>
rename 1247.misc.rst to changelog/1247.misc.rst</li>
<li><a
href="https://github.com/pypa/twine/commit/d94a4750883dc7cc92b2cbbb3970895303af216d"><code>d94a475</code></a>
fix(tests): update expected error message</li>
<li><a
href="https://github.com/pypa/twine/commit/c1c02d13d00f496339a86cd4f40d9f7e6926c0e4"><code>c1c02d1</code></a>
Remove --skip-existing support for non-PyPI indices</li>
<li><a
href="https://github.com/pypa/twine/commit/a24d308bb7fa5304ede3414747a939c7929d4c76"><code>a24d308</code></a>
Set trusted publishing logging to INFO/WARN (<a
href="https://redirect.github.com/pypa/twine/issues/1247">#1247</a>)</li>
<li><a
href="https://github.com/pypa/twine/commit/becf1a8ffe6ebd404aea9d7aeae1f756aeba57ba"><code>becf1a8</code></a>
Fix py3.9 mypy error in <code>__init__</code> around
<code>PackageMetadata</code></li>
<li>Additional commits viewable in <a
href="https://github.com/pypa/twine/compare/6.1.0...6.2.0">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=twine&package-manager=pip&previous-version=6.1.0&new-version=6.2.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[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.

4 participants