From 3e207d443b88951d7458b5b4067f97a95b6a74b2 Mon Sep 17 00:00:00 2001 From: John Plaisted Date: Tue, 3 Nov 2020 10:01:39 -0800 Subject: [PATCH] fix: start turning on werror for code. I've touched up a few easy to fix places, but have yet to fix everything / all modules. Might be wise to do this on a module by module basis. --- build.gradle | 22 +++++++++++++++++++ .../com/linkedin/common/urn/TupleKey.java | 14 ++++++++---- .../linkedin/testing/urn/BaseUrnCoercer.java | 1 + .../metadata/validator/SnapshotValidator.java | 8 ++++--- .../metadata/validator/ValidationUtils.java | 7 ++++-- 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/build.gradle b/build.gradle index f35a6f8e5..6b14835c5 100644 --- a/build.gradle +++ b/build.gradle @@ -55,12 +55,34 @@ project.ext.externalDependency = [ apply plugin: 'com.diffplug.spotless' apply plugin: 'org.shipkit.shipkit-auto-version' +// TODO expand this to all projects and then delete this allow list. This list is letting us fix errors over time rather +// than in one big change. +def wErrorProjects = [ + project(':core-models'), + // project(':dao-api'), + // project(':dao-api-impl'), + // project(':restli-resources'), + project(':testing'), + project(':validators') +] + allprojects { group = "com.linkedin.datahub-gma" apply plugin: 'idea' apply plugin: 'eclipse' apply plugin: 'checkstyle' + + gradle.projectsEvaluated { + if (wErrorProjects.contains(project)) { + tasks.withType(JavaCompile) { + options.compilerArgs << "-Xlint:all" << "-Werror" << + "-Xlint:-deprecation" << // TODO + "-Xlint:-processing" << // TODO we have annotations like @Nonnull that need a processor + "-Xlint:-serial" // I don't think we care about having custom Exception subclasses be serializable... + } + } + } } if (!project.hasProperty('disableShipkit')) { diff --git a/core-models/src/main/javaPegasus/com/linkedin/common/urn/TupleKey.java b/core-models/src/main/javaPegasus/com/linkedin/common/urn/TupleKey.java index 10183ecbd..c26e0d257 100644 --- a/core-models/src/main/javaPegasus/com/linkedin/common/urn/TupleKey.java +++ b/core-models/src/main/javaPegasus/com/linkedin/common/urn/TupleKey.java @@ -155,10 +155,7 @@ public T getAs(int index, Class clazz) { } else if (Long.TYPE.equals(clazz) || Long.class.equals(clazz)) { result = Long.valueOf(value); } else if (Enum.class.isAssignableFrom(clazz)) { - final Class enumClazz = clazz.asSubclass(Enum.class); - @SuppressWarnings("unchecked") - Enum enumValue = Enum.valueOf(enumClazz, value); - result = enumValue; + result = getEnumValue(clazz, value); } else if (DataTemplateUtil.hasCoercer(clazz)) { result = DataTemplateUtil.coerceOutput(value, clazz); } else { @@ -169,6 +166,15 @@ public T getAs(int index, Class clazz) { return rv; } + /** + * Helper method to capture E. + */ + private > Enum getEnumValue(Class clazz, String value) { + @SuppressWarnings("unchecked") + final Class enumClazz = (Class) clazz.asSubclass(Enum.class); + return Enum.valueOf(enumClazz, value); + } + public int size() { return _tuple.size(); } diff --git a/testing/test-models/src/main/javaPegasus/com/linkedin/testing/urn/BaseUrnCoercer.java b/testing/test-models/src/main/javaPegasus/com/linkedin/testing/urn/BaseUrnCoercer.java index 0a25bce8d..d2454c430 100644 --- a/testing/test-models/src/main/javaPegasus/com/linkedin/testing/urn/BaseUrnCoercer.java +++ b/testing/test-models/src/main/javaPegasus/com/linkedin/testing/urn/BaseUrnCoercer.java @@ -14,6 +14,7 @@ public Object coerceInput(T object) throws ClassCastException { return object.toString(); } + @SuppressWarnings("unchecked") public T coerceOutput(Object object) throws TemplateOutputCastException { try { return (T) Urn.createFromString((String) object); diff --git a/validators/src/main/java/com/linkedin/metadata/validator/SnapshotValidator.java b/validators/src/main/java/com/linkedin/metadata/validator/SnapshotValidator.java index 42753a1cb..a4a9eefea 100644 --- a/validators/src/main/java/com/linkedin/metadata/validator/SnapshotValidator.java +++ b/validators/src/main/java/com/linkedin/metadata/validator/SnapshotValidator.java @@ -1,5 +1,6 @@ package com.linkedin.metadata.validator; +import com.linkedin.common.urn.Urn; import com.linkedin.data.schema.ArrayDataSchema; import com.linkedin.data.schema.DataSchema; import com.linkedin.data.schema.RecordDataSchema; @@ -60,10 +61,11 @@ public static void validateSnapshotSchema(@Nonnull Class snapshotClasses) { - Set urnClasses = new HashSet<>(); + public static void validateUniqueUrn(@Nonnull Collection> snapshotClasses) { + final Set> urnClasses = new HashSet<>(); snapshotClasses.forEach(snapshotClass -> { - Class urnClass = ValidationUtils.getUrnClass(ValidationUtils.getRecordSchema(snapshotClass).getField("urn")); + final Class urnClass = + ValidationUtils.getUrnClass(ValidationUtils.getRecordSchema(snapshotClass).getField("urn")); if (urnClasses.contains(urnClass)) { ValidationUtils.invalidSchema("URN class %s in %s has already been claimed by another snapshot.", urnClass, snapshotClass); diff --git a/validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java b/validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java index e16bb5319..ef3714a25 100644 --- a/validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java +++ b/validators/src/main/java/com/linkedin/metadata/validator/ValidationUtils.java @@ -95,9 +95,12 @@ public static boolean isValidUrnField(@Nonnull RecordDataSchema.Field field, @No /** * Returns the Java class for an URN typeref field. */ - public static Class getUrnClass(@Nonnull RecordDataSchema.Field field) { + public static Class getUrnClass(@Nonnull RecordDataSchema.Field field) { try { - return Class.forName(((DataMap) field.getType().getProperties().get("java")).getString("class")); + @SuppressWarnings("unchecked") + final Class clazz = + (Class) Class.forName(((DataMap) field.getType().getProperties().get("java")).getString("class")); + return clazz; } catch (ClassNotFoundException e) { throw new RuntimeException(e); }