From adfa6a50adf5cb297565bb218fd7dfce99765079 Mon Sep 17 00:00:00 2001 From: Rico Yao Date: Wed, 13 Mar 2019 11:58:47 -0700 Subject: [PATCH 1/2] For Boolean, Double and Integer properties, getters should be NonNull Otherwise callers will always have to defend against null values, which is a big pain. We have isVARIABLESet() methods if developers want to check if a property was actually set or not. --- Examples/Java/Sources/Everything.java | 12 ++--- Examples/Java/Sources/Image.java | 8 +-- .../Java/Sources/VariableSubtitution.java | 16 +++--- Sources/Core/JavaIR.swift | 43 +++++++++++---- Sources/Core/JavaModelRenderer.swift | 53 +++++++++++++++---- 5 files changed, 92 insertions(+), 40 deletions(-) diff --git a/Examples/Java/Sources/Everything.java b/Examples/Java/Sources/Everything.java index 9e18e231..5a94110e 100644 --- a/Examples/Java/Sources/Everything.java +++ b/Examples/Java/Sources/Everything.java @@ -288,8 +288,8 @@ public int hashCode() { return this.arrayProp; } - public @Nullable Boolean getBooleanProp() { - return this.booleanProp; + public @NonNull Boolean getBooleanProp() { + return this.booleanProp == null ? Boolean.FALSE : this.booleanProp; } public @Nullable Date getDateProp() { @@ -300,8 +300,8 @@ public int hashCode() { return this.intEnum; } - public @Nullable Integer getIntProp() { - return this.intProp; + public @NonNull Integer getIntProp() { + return this.intProp == null ? 0 : this.intProp; } public @Nullable List getListPolymorphicValues() { @@ -356,8 +356,8 @@ public int hashCode() { return this.mapWithPrimitiveValues; } - public @Nullable Double getNumberProp() { - return this.numberProp; + public @NonNull Double getNumberProp() { + return this.numberProp == null ? 0 : this.numberProp; } public @Nullable User getOtherModelProp() { diff --git a/Examples/Java/Sources/Image.java b/Examples/Java/Sources/Image.java index 320d62cf..b874c746 100644 --- a/Examples/Java/Sources/Image.java +++ b/Examples/Java/Sources/Image.java @@ -88,16 +88,16 @@ public int hashCode() { width); } - public @Nullable Integer getHeight() { - return this.height; + public @NonNull Integer getHeight() { + return this.height == null ? 0 : this.height; } public @Nullable String getUrl() { return this.url; } - public @Nullable Integer getWidth() { - return this.width; + public @NonNull Integer getWidth() { + return this.width == null ? 0 : this.width; } public boolean getHeightIsSet() { diff --git a/Examples/Java/Sources/VariableSubtitution.java b/Examples/Java/Sources/VariableSubtitution.java index 8f57080a..1c17ea6c 100644 --- a/Examples/Java/Sources/VariableSubtitution.java +++ b/Examples/Java/Sources/VariableSubtitution.java @@ -94,20 +94,20 @@ public int hashCode() { newProp); } - public @Nullable Integer getAllocProp() { - return this.allocProp; + public @NonNull Integer getAllocProp() { + return this.allocProp == null ? 0 : this.allocProp; } - public @Nullable Integer getCopyProp() { - return this.copyProp; + public @NonNull Integer getCopyProp() { + return this.copyProp == null ? 0 : this.copyProp; } - public @Nullable Integer getMutableCopyProp() { - return this.mutableCopyProp; + public @NonNull Integer getMutableCopyProp() { + return this.mutableCopyProp == null ? 0 : this.mutableCopyProp; } - public @Nullable Integer getNewProp() { - return this.newProp; + public @NonNull Integer getNewProp() { + return this.newProp == null ? 0 : this.newProp; } public boolean getAllocPropIsSet() { diff --git a/Sources/Core/JavaIR.swift b/Sources/Core/JavaIR.swift index de6bb191..c286df90 100644 --- a/Sources/Core/JavaIR.swift +++ b/Sources/Core/JavaIR.swift @@ -26,10 +26,24 @@ struct JavaModifier: OptionSet { } } -enum JavaAnnotation: String { - case override = "Override" - case nullable = "Nullable" - case nonnull = "NonNull" +enum JavaAnnotation: Hashable { + case override + case nullable + case nonnull + case serializedName(name: String) + + var rendered: String { + switch self { + case .override: + return "Override" + case .nullable: + return "Nullable" + case .nonnull: + return "NonNull" + case let .serializedName(name): + return "SerializedName(\"\(name)\")" + } + } } public struct JavaIR { @@ -38,16 +52,19 @@ public struct JavaIR { let modifiers: JavaModifier let body: [String] let signature: String + let throwableExceptions: [String] func render() -> [String] { // HACK: We should actually have an enum / optionset that we can check for abstract, static, ... - let annotationLines = annotations.map { "@\($0.rawValue)" } + let annotationLines = annotations.map { "@\($0.rendered)" } + let throwsString = throwableExceptions.isEmpty ? "" : " throws " + throwableExceptions.joined(separator: ", ") + if modifiers.contains(.abstract) { - return annotationLines + ["\(modifiers.render()) \(signature);"] + return annotationLines + ["\(modifiers.render()) \(signature)\(throwsString);"] } - var toRender = annotationLines + ["\(modifiers.render()) \(signature) {"] + var toRender = annotationLines + ["\(modifiers.render()) \(signature)\(throwsString) {"] if !body.isEmpty { toRender.append(-->body) @@ -59,7 +76,7 @@ public struct JavaIR { } public struct Property { - let annotations: Set + let annotations: Set let modifiers: JavaModifier let type: String let name: String @@ -68,7 +85,7 @@ public struct JavaIR { func render() -> String { var prop = "" if !annotations.isEmpty { - prop.append(annotations.map { "@\($0)" }.joined(separator: " ") + " ") + prop.append(annotations.map { "@\($0.rendered)" }.joined(separator: " ") + " ") } prop.append("\(modifiers.render()) \(type) \(name)") if !initialValue.isEmpty { @@ -80,7 +97,11 @@ public struct JavaIR { } static func method(annotations: Set = [], _ modifiers: JavaModifier, _ signature: String, body: () -> [String]) -> JavaIR.Method { - return JavaIR.Method(annotations: annotations, modifiers: modifiers, body: body(), signature: signature) + return JavaIR.Method(annotations: annotations, modifiers: modifiers, body: body(), signature: signature, throwableExceptions: []) + } + + static func methodThatThrows(annotations: Set = [], _ modifiers: JavaModifier, _ signature: String, _ throwableExceptions: [String], body: () -> [String]) -> JavaIR.Method { + return JavaIR.Method(annotations: annotations, modifiers: modifiers, body: body(), signature: signature, throwableExceptions: throwableExceptions) } static func ifBlock(condition: String, body: () -> [String]) -> String { @@ -181,7 +202,7 @@ public struct JavaIR { let extendsStmt = extends.map { " extends \($0) " } ?? "" - var lines = annotations.map { "@\($0.rawValue)" } + [ + var lines = annotations.map { "@\($0.rendered)" } + [ "\(modifiers.render()) class \(name)\(extendsStmt)\(implementsStmt) {", ] diff --git a/Sources/Core/JavaModelRenderer.swift b/Sources/Core/JavaModelRenderer.swift index 5eb57e52..5885addc 100644 --- a/Sources/Core/JavaModelRenderer.swift +++ b/Sources/Core/JavaModelRenderer.swift @@ -61,7 +61,7 @@ public struct JavaModelRenderer: JavaFileRenderer { func renderModelProperties(modifiers _: JavaModifier = [.private]) -> [[JavaIR.Property]] { let props = transitiveProperties.map { param, schemaObj in - JavaIR.Property(annotations: ["SerializedName(\"\(param)\")"], modifiers: [.private], type: self.typeFromSchema(param, schemaObj), name: param.snakeCaseToPropertyName(), initialValue: "") + JavaIR.Property(annotations: [.serializedName(name: param)], modifiers: [.private], type: self.typeFromSchema(param, schemaObj), name: param.snakeCaseToPropertyName(), initialValue: "") } let bits = JavaIR.Property(annotations: [], modifiers: [.private], type: "int", name: "_bits", initialValue: "0") @@ -76,11 +76,40 @@ public struct JavaModelRenderer: JavaFileRenderer { return [props, bitmasks, [bits]] } - func renderModelGetters(modifiers: JavaModifier = [.public]) -> [JavaIR.Method] { + func propertyGetterForParam(param: String, schemaObj: SchemaObjectProperty) -> JavaIR.Method { + + let propertyName = param.snakeCaseToPropertyName() + let capitalizedPropertyName = param.snakeCaseToCapitalizedPropertyName() + + // For Booleans, Integers and Doubles, make the getter method @NonNull and squash to a default value if necessary. + // This makes callers less susceptible to null pointer exceptions. + switch schemaObj.schema { + case .boolean: + return JavaIR.method([.public], "@NonNull Boolean get" + capitalizedPropertyName + "()") { [ + "return this." + propertyName + " == null ? Boolean.FALSE : this." + propertyName + ";", + ] + } + case .integer: + return JavaIR.method([.public], "@NonNull Integer get" + capitalizedPropertyName + "()") { [ + "return this." + propertyName + " == null ? 0 : this." + propertyName + ";", + ] + } + case .float: + return JavaIR.method([.public], "@NonNull Double get" + capitalizedPropertyName + "()") { [ + "return this." + propertyName + " == null ? 0 : this." + propertyName + ";", + ] + } + default: + return JavaIR.method([.public], typeFromSchema(param, schemaObj) + " get" + capitalizedPropertyName + "()") { [ + "return this." + propertyName + ";", + ] + } + } + } + + func renderModelPropertyGetters() -> [JavaIR.Method] { let getters = transitiveProperties.map { param, schemaObj in - JavaIR.method(modifiers, self.typeFromSchema(param, schemaObj) + " get" + param.snakeCaseToCapitalizedPropertyName() + "()") { [ - "return this." + param.snakeCaseToPropertyName() + ";", - ] } + propertyGetterForParam(param: param, schemaObj: schemaObj) } return getters } @@ -164,7 +193,7 @@ public struct JavaModelRenderer: JavaFileRenderer { func renderBuilderProperties(modifiers _: JavaModifier = [.private]) -> [[JavaIR.Property]] { let props = transitiveProperties.map { param, schemaObj in - JavaIR.Property(annotations: ["SerializedName(\"\(param)\")"], modifiers: [.private], type: self.typeFromSchema(param, schemaObj), name: param.snakeCaseToPropertyName(), initialValue: "") + JavaIR.Property(annotations: [.serializedName(name: param)], modifiers: [.private], type: self.typeFromSchema(param, schemaObj), name: param.snakeCaseToPropertyName(), initialValue: "") } let bits = JavaIR.Property(annotations: [], modifiers: [.private], type: "int", name: "_bits", initialValue: "0") @@ -215,18 +244,20 @@ public struct JavaModelRenderer: JavaFileRenderer { "this.elementTypeAdapter = gson.getAdapter(JsonElement.class);", ] } - let write = JavaIR.method( + let write = JavaIR.methodThatThrows( annotations: [JavaAnnotation.override], [.public], - "void write(JsonWriter writer, " + className + " value) throws IOException" + "void write(JsonWriter writer, " + className + " value)", + ["IOException"] ) { [ "this.delegateTypeAdapter.write(writer, value);", ] } - let read = JavaIR.method( + let read = JavaIR.methodThatThrows( annotations: [JavaAnnotation.override], [.public], - className + " read(JsonReader reader) throws IOException" + className + " read(JsonReader reader)", + ["IOException"] ) { [ "JsonElement tree = this.elementTypeAdapter.read(reader);", className + " model = this.delegateTypeAdapter.fromJsonTree(tree);", @@ -378,7 +409,7 @@ public struct JavaModelRenderer: JavaFileRenderer { self.renderModelEquals(), self.renderModelHashCode(), ] + - renderModelGetters() + + renderModelPropertyGetters() + renderModelIsSetCheckers(), enums: enumProps, innerClasses: [ From 52a45b0fd8ad36b125546a5accaee5d82888e0e6 Mon Sep 17 00:00:00 2001 From: Rico Yao Date: Wed, 13 Mar 2019 13:41:39 -0700 Subject: [PATCH 2/2] make lint,format --- Sources/Core/JavaIR.swift | 6 +++--- Sources/Core/JavaModelRenderer.swift | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Sources/Core/JavaIR.swift b/Sources/Core/JavaIR.swift index c286df90..8a93d197 100644 --- a/Sources/Core/JavaIR.swift +++ b/Sources/Core/JavaIR.swift @@ -31,7 +31,7 @@ enum JavaAnnotation: Hashable { case nullable case nonnull case serializedName(name: String) - + var rendered: String { switch self { case .override: @@ -59,7 +59,7 @@ public struct JavaIR { let annotationLines = annotations.map { "@\($0.rendered)" } let throwsString = throwableExceptions.isEmpty ? "" : " throws " + throwableExceptions.joined(separator: ", ") - + if modifiers.contains(.abstract) { return annotationLines + ["\(modifiers.render()) \(signature)\(throwsString);"] } @@ -99,7 +99,7 @@ public struct JavaIR { static func method(annotations: Set = [], _ modifiers: JavaModifier, _ signature: String, body: () -> [String]) -> JavaIR.Method { return JavaIR.Method(annotations: annotations, modifiers: modifiers, body: body(), signature: signature, throwableExceptions: []) } - + static func methodThatThrows(annotations: Set = [], _ modifiers: JavaModifier, _ signature: String, _ throwableExceptions: [String], body: () -> [String]) -> JavaIR.Method { return JavaIR.Method(annotations: annotations, modifiers: modifiers, body: body(), signature: signature, throwableExceptions: throwableExceptions) } diff --git a/Sources/Core/JavaModelRenderer.swift b/Sources/Core/JavaModelRenderer.swift index 5885addc..0d02a2a1 100644 --- a/Sources/Core/JavaModelRenderer.swift +++ b/Sources/Core/JavaModelRenderer.swift @@ -77,10 +77,9 @@ public struct JavaModelRenderer: JavaFileRenderer { } func propertyGetterForParam(param: String, schemaObj: SchemaObjectProperty) -> JavaIR.Method { - let propertyName = param.snakeCaseToPropertyName() let capitalizedPropertyName = param.snakeCaseToCapitalizedPropertyName() - + // For Booleans, Integers and Doubles, make the getter method @NonNull and squash to a default value if necessary. // This makes callers less susceptible to null pointer exceptions. switch schemaObj.schema {