Skip to content

Conversation

@osa1
Copy link
Member

@osa1 osa1 commented Oct 22, 2025

For {extension,non-extension} {singular,repeated} fields, check range and type
checks.

All fields with range checks are tested. For type and nullability checks we
only use a message and a bool field, as there are too many types of fields
and most of them use the same validation function.

Fields with range checks are not tested for type and nullability checks. To
check whether a value is within range we have to use as a non-null int or
double, so if the value being checked doesn't have the right type the range
check function will throw anyway.

This reveals two inconsistencies in the library:

  • When type of a non-repeated extension value is not right we throw an argument
    error instead of a type error.

  • setExtension takes an Object (not nullable) as an argument but
    addExtension takes Object? (nullable).

I tested accordingly instead of trying to fix the inconsistencies as fixing
them will break downstream.

@osa1 osa1 requested a review from sigurdm October 22, 2025 10:58
@osa1 osa1 marked this pull request as ready for review October 22, 2025 10:58
@osa1 osa1 force-pushed the field_validation_tests branch from 7e978c8 to cbefc59 Compare October 22, 2025 12:00
@osa1 osa1 merged commit a77f9a6 into google:master Oct 23, 2025
23 checks passed
@osa1 osa1 deleted the field_validation_tests branch October 23, 2025 09:31
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Oct 27, 2025
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/98d03ad..6d1aa6f):
  6d1aa6f5  2025-10-23  Sam Rawlins  Refactor Locatable into an interface, HasLocation (dart-lang/dartdoc#4118)
  f82cd35d  2025-10-23  Sam Rawlins  Remove unused extension method, replaced in analyzer 8.4.0 (dart-lang/dartdoc#4119)

protobuf (https://github.com/dart-lang/protobuf/compare/78cf743..7db0784):
  7db0784  2025-10-24  Ömer Sinan Ağacan  Avoid redundant nullability checks when updating repeated fields (google/protobuf.dart#1069)
  a77f9a6  2025-10-23  Ömer Sinan Ağacan  More field validation tests (google/protobuf.dart#1068)
  27730db  2025-10-21  Ömer Sinan Ağacan  Move PbMap to its own library (google/protobuf.dart#1066)
  9b4c46f  2025-10-21  Ömer Sinan Ağacan  More clearExtension tests: (google/protobuf.dart#1064)
  5a44489  2025-10-21  Ömer Sinan Ağacan  Move PbList to its own library (google/protobuf.dart#1063)

test (https://github.com/dart-lang/test/compare/8083c8f..5855358):
  58553580  2025-10-24  Nate Bosch  Add a sentence to clarify "Platform" (dart-lang/test#2550)

tools (https://github.com/dart-lang/tools/compare/5fe6ee6..e0cc0bc):
  e0cc0bcc  2025-10-27  Morgan :)  Test DirectoryWatcher exception on missing path. (dart-lang/tools#2224)
  e6ce99da  2025-10-24  Morgan :)  Test new link to directory races, fix for them (dart-lang/tools#2223)
  9053fae7  2025-10-24  Morgan :)  Check file sizes as well as "last modified" times. (dart-lang/tools#2221)

Change-Id: Ica6937ba0af21c5ba435e97bd0673fbe7b3043cc
Tested: update test goldens
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/457800
Auto-Submit: Devon Carew <devoncarew@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.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