Skip to content

Conversation

@wwwjfy
Copy link
Contributor

@wwwjfy wwwjfy commented Feb 9, 2017

PR checklist

  • Read the contribution guildelines.
  • Ran the shell/batch script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)
  • Filed the PR against the correct branch: master for non-breaking changes and 2.3.0 branch for breaking (non-backward compatible) changes.

Description of the PR

As all other the fields can be null, I think an enumeration field can also be null. The users can decide whether that's a problem. This is to be consistent with previous behaviors.
This was introduced in #4547 (@philippelatulippe)

The real change is only one line, but I cannot help but fix format issues.

@wing328 wing328 added this to the v2.2.2 milestone Feb 13, 2017
@philippelatulippe
Copy link
Contributor

Good catch, if the field is null it will return a typeMismatch, which is incorrect for decodeOptional(). But will this also return nil in the case that the field in the data is of the wrong type? I think the best behaviour is to have type mismatches return an error and only null or missing fields return nil. Perhaps this needs to be debated? The pros are that errors can be caught early, but the parser is less permissive.

@wwwjfy
Copy link
Contributor Author

wwwjfy commented Feb 17, 2017

Thanks for reviewing the code. You're right, it shouldn't return success if the type mismatched.
My intention is to return success when it's null, missing the case of mismatched type.
I'll commit a fix later.

@wing328
Copy link
Contributor

wing328 commented Feb 17, 2017

cc @jaz-ah @jgavris

@wwwjfy
Copy link
Contributor Author

wwwjfy commented Feb 17, 2017

I pushed a fix.

Another similar place (which is left intact) is https://github.com/swagger-api/swagger-codegen/pull/4762/files#diff-940535e66d162a893df844fd9b6b45ffL202
The function signature is
static func decodeOptional<T: RawRepresentable, U: AnyObject where T.RawValue == U>(clazz: T, source: AnyObject) -> Decoded<T?>

I don't know when this function will be called. I'm a beginner in Swift. Isn't this case already covered in the function without U: AnyObject?

@jaz-ah
Copy link
Contributor

jaz-ah commented Feb 17, 2017

+1 thx for the contribution @wwwjfy

@wwwjfy wwwjfy force-pushed the swift3-nullable-enum branch from da908c3 to c2ac613 Compare February 18, 2017 02:09
@wing328 wing328 modified the milestones: v2.2.2, v2.2.3 Feb 22, 2017
@wing328 wing328 merged commit f968316 into swagger-api:2.3.0 Mar 12, 2017
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.

4 participants