Skip to content

feat: use proto presence information to inform nullability#58

Closed
devoncarew wants to merge 6 commits intogoogleapis:mainfrom
devoncarew:null_safety
Closed

feat: use proto presence information to inform nullability#58
devoncarew wants to merge 6 commits intogoogleapis:mainfrom
devoncarew:null_safety

Conversation

@devoncarew
Copy link
Copy Markdown
Contributor

An update to #43; this is still a draft PR.

  • now use whether a field is in a oneof to help inform nullability

print('<No textual response>');
} else {
print(parts.map((p) => p.text ?? '').join(''));
print(parts.map((p) => p.text).join(''));
Copy link
Copy Markdown
Contributor

@brianquinlan brianquinlan Oct 30, 2025

Choose a reason for hiding this comment

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

Is this example right? The generated Part has a nullable text field:

final class Part extends ProtoMessage {
  static const String fullyQualifiedName =
      'google.ai.generativelanguage.v1beta.Part';

  /// Inline text.
  final String? text;

Which is what I'd expect because it is part of a OneOf.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No - good catch. text is nullable, but we're assuming has content based on the context of the sample. I added a 'bang' (!).

print(parts.map((p) => p.text!).join(''));

devoncarew added a commit that referenced this pull request Oct 31, 2025
- add `isNotDefault` extension methods for core proto types

This is the hand-written portions of
#58. I'll follow
this up w/ generator changes (in the librarian repo), and then a PR back
to this repo that will re-generate the packages and update test and
example code (the rest of #58).

Happy to bike-shed on the extension method name(s). It's `isNotDefault`
now, but could be something like `notDefault`, ... .

cc @natebosch
Copy link
Copy Markdown
Contributor

@brianquinlan brianquinlan left a comment

Choose a reason for hiding this comment

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

This is going to be a pain to merge.

For package versions, I think that the easiest thing to do is:

  1. remove all of the version = from all .sidekick.toml except the one at the root
  2. update the root .sidekick.toml "# Generated packages section" such that all of the version constraints are '^0.2.0'

Not only is that expedient, but I think it is also correct ;-)

Let me know if you want me to do the merge work.


final result = await service.generateContent(request);
final parts = result.candidates?[0].content?.parts;
final parts = result.candidates.first.content?.parts;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
final parts = result.candidates.first.content?.parts;
final parts = result.candidates.firstOrNull?.content?.parts;

final parts = result.candidates.first.content?.parts;
if (parts != null) {
parts.forEach((p) => stdout.write(p.text ?? ''));
parts.forEach((p) => stdout.write(p.text));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
parts.forEach((p) => stdout.write(p.text));
parts.forEach((p) => stdout.write(p.text!));

I think? Could you rerun these examples.


final result = await service.generateContent(request);
final parts = result.candidates?[0].content?.parts;
final parts = result.candidates.first.content?.parts;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
final parts = result.candidates.first.content?.parts;
final parts = result.candidates.firstOrNull?.content?.parts;


final result = await service.generateContent(request);
final parts = result.candidates?[0].content?.parts;
final parts = result.candidates.first.content?.parts;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
final parts = result.candidates.first.content?.parts;
final parts = result.candidates.firstOrNull?.content?.parts;

@devoncarew
Copy link
Copy Markdown
Contributor Author

This is going to be a pain to merge.

What I was thinking of - once the upstream sidekick change lands - was to create a new PR w/ the latest generated code. I.e., not trying to update this PR. I'll then make the same changes to the examples and tests that this PR has. Those were all straightforward - just addressing issues reported by the type system.

devoncarew added a commit to googleapis/librarian that referenced this pull request Oct 31, 2025
Use proto presence info to generate null-safe code:

- `annotateField` updated to use information from proto optional
annotations
- toJson() implementations write values if they're nullable and have a
non-null value, or if they're not nullable and have a non-default value
for that type
- fromJson parses a value for parse a value for nullable fields if one
is given; it will parse any value for non-null field or fall back to
using the default value for that type
- properly dereference null values when referring to request fields for
use in url paths
- enums definitions now get a default value (`EnumClass.$default`)
- removed the older annotate.requiredFields and tests (based on usage in
service calls)
- closes googleapis/google-cloud-dart#24

Tests are updated for `buildQueryLines()` to include optional fields.
Tests were added for the `createFromJsonLine()` and `createToJsonLine()`
methods; previously we got our testing here from being on the same repo
as the dart code, and running the dart unit tests on the generator's
putput.

In-lined from the PR:

>	- proto 3 fields default to implicit presence
>	- the 'optional' keyword changes a field to explicit presence
>	- types like lists (repeated) and maps are always implicit presence
>	
> Explicit presence means that you can know whether the user set a value
or
> not. Implicit presence means you can always retrieve a value; if one
had
>	not been set, you'll see the default value for that type.
>	
> We translate explicit presence (a optional annotation) to using a
nullable
> type for that field. We translate implicit presence (always returning
some
>	value) to a non-null type.
>	
>	Some short-hand:
>	  - optional == explicit == nullable
>	  - implicit == non-nullable
>	  - lists and maps == implicit == non-nullable
>	  - singular message == explicit == nullable

Sample output can be seen at
googleapis/google-cloud-dart#58.
@devoncarew devoncarew closed this Oct 31, 2025
devoncarew added a commit that referenced this pull request Oct 31, 2025
Update the packages here using the null-safety generator change:

- a follow up to googleapis/librarian#2726
- update sidekick version reference
- re-generate the code
- update the hand-written code (part files, examples, and tests)

Previously as a draft at
#58; done as
separate commits to make review a little easier.

@brianquinlan, this does not include sidekick / dart package version
changes. I assume we'll want to bump everything (to 0.2.0?). Not sure if
we're bumping on changes or on publish (or don't have a specific process
yet).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants