Skip to content

Conversation

helin24
Copy link
Contributor

@helin24 helin24 commented Feb 4, 2022

The java generation for type (string|Null)[] was creating @return one of <code>List<String></code> or <code>ElementList<Null></code>

see example:

@SuppressWarnings({"WeakerAccess", "unused"})
public class UriList extends Response {
 
  public UriList(JsonObject json) {
    super(json);
  }
 
  /**
   * A list of URIs.
   *
   * @return one of <code>List<String></code> or <code>ElementList<Null></code>
   */
  public Object getUris() {
    final JsonObject elem = (JsonObject)json.get("uris");
    if (elem == null) return null;
 
    if (elem.get("type").getAsString().equals("String")) return new String(elem);
    if (elem.get("type").getAsString().equals("Null")) return new Null(elem);
    return null;
  }

My change is the simplest thing I could think of, just making the return type List<String> instead and allowing some of the elements to be null. But I'm happy to implement something else if someone has a suggestion for a better type (along with how to get the generate code to produce it)

@helin24 helin24 requested review from bkonyi and devoncarew February 4, 2022 17:52
@bkonyi
Copy link
Contributor

bkonyi commented Feb 4, 2022

I don't think I can merge GitHub PRs directly into the SDK. We'll probably need to review this through Gerrit (the command to create a Gerrit review is git cl upload).

@helin24
Copy link
Contributor Author

helin24 commented Feb 4, 2022

Oh I see, I will try that. Thanks!

@devoncarew
Copy link
Member

I think it can be reviewed and merged through here?

https://dart-review.googlesource.com/c/sdk/+/231740

@bkonyi
Copy link
Contributor

bkonyi commented Feb 4, 2022

I think it can be reviewed and merged through here?

https://dart-review.googlesource.com/c/sdk/+/231740

Ah, yup! That should work.

@copybara-service copybara-service bot closed this in 43d124c Feb 4, 2022
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.

3 participants