GoogleCloudPlatform / cloud-opensource-java

Tools for detecting and avoiding linkage errors in GCP open source projects
Apache License 2.0
155 stars 74 forks source link

Configuration file to express existing errors #1167

Open suztomo opened 4 years ago

suztomo commented 4 years ago

Similar to https://github.com/GoogleCloudPlatform/cloud-opensource-java/issues/940 but intended to suppress (or ignore) existing linkage errors, rather than special cases.

From Luke's comment in BEAM-9206:

Typically I have seen people use an ignore list that is hardcoded somewhere such as the ones found in spotbugs and the test/tool runs and compares against that. This way the the contributor can update the existing list and get the test to pass and the reviewer can see what the edits were so the CL can "pass" all tests.

Case of Spotbugs-filter

Beam's use case of spotbugs-filter.xml is https://github.com/apache/beam/blob/master/sdks/java/build-tools/src/main/resources/beam/spotbugs-filter.xml

...
  <Match>
    <Class name="org.apache.beam.sdk.coders.AvroCoder$SerializableSchemaSupplier"/>
    <Field name="schema"/>
    <Bug pattern="SE_BAD_FIELD"/>
    <!--
    writeReplace makes this object serializable. This is a limitation of FindBugs as discussed here:
    http://stackoverflow.com/questions/26156523/is-writeobject-not-neccesary-using-the-serialization-proxy-pattern
    -->
  </Match>
...
  <Match>
    <!--
      This is a false positive. Spotbugs does not recognize the use of try-with-resources, so it thinks that
      the connection is not correctly closed.
    -->
    <Or>
      <And>
        <Class name="org.apache.beam.sdk.io.jdbc.JdbcIO$ReadFn"/>
        <Method name="processElement"/>
      </And>
      <And>
        <Class name="org.apache.beam.sdk.io.jdbc.JdbcIO$ReadRows"/>
        <Method name="inferBeamSchema"/>
      </And>
    </Or>

    <Bug pattern="OBL_UNSATISFIED_OBLIGATION"/>
  </Match>
suztomo commented 4 years ago

Idea of plain text file. This cannot have structure but it works fine if it's just for the targets.

com.google.guava:guava:25.1-jre/com.google.common.collection.ImmutableList/size
com.google.guava:guava:*/**
*weld-osgi-bundle*/**
*/com.github.luben.zstd.ZstdInputStream
*/com.github.luben.zstd.ZstdOutputStream
*/org.apache.beam.vendor.bytebuddy.v1_9_3.net.bytebuddy.jar.asm.commons.ModuleHashesAttribute

The information written in the file should be independent from the data types in our Java code. The information may have source and target.

Question

suztomo commented 4 years ago

Source and target should be present. Thinking how to represent the special case in Linkage Checker's suppression logic

Example (if we use XML)

<LinkageError>
  <Source>
    <Class name="org.apache.beam.sdk.io.jdbc.JdbcIO$ReadFn">
  </Source>
  <Target>
    <Artifact coordinates="com.google.guava:guava:25.1-jre">
  </Target>
</LinkageError>

A LinkageError element represents a matcher that takes at least one of Source and Target elements.

A Source or Target element takes one of Class, Artifact, and Package element.

elharo commented 4 years ago

This looks good. Should the target be an artifact or a class or both? I think we'll also need the specific method or field being linked to.