Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HHH-15846 Option to add @SuppressFBWarnings annotation on generated code #5790

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hibernate-core/hibernate-core.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ dependencies {
}
testImplementation "joda-time:joda-time:2.3"
testImplementation dbLibs.h2
testImplementation testLibs.archunitJUnit4

testRuntimeOnly libs.byteBuddy
testRuntimeOnly testLibs.weld
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ public boolean hasLazyLoadableAttributes(TypeDescription classDescriptor) {
return enhancementContext.hasLazyLoadableAttributes( new UnloadedTypeDescription( classDescriptor ) );
}

public boolean addSuppressFBWarnings(TypeDescription classDescriptor) {
return enhancementContext.addSuppressFBWarnings( new UnloadedTypeDescription( classDescriptor ) );
}

public boolean isPersistentField(AnnotatedFieldDescription field) {
return enhancementContext.isPersistentField( field );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,23 @@ static Collection<Object> getter(Object self) {

}

/**
* Used to suppress <a href="https://spotbugs.readthedocs.io">SpotBugs</a> warnings.
*/
@Retention(RetentionPolicy.CLASS)
@interface SuppressFBWarnings {
/**
* @see "https://spotbugs.readthedocs.io/en/latest/bugDescriptions.html"
*/
String[] value();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be able to remove value(). It works fine without it.


/**
* Reason why the warning is suppressed. Use a SpotBugs issue id where appropriate.
*/
String justification();
}


// mapping to get private field from superclass by calling the enhanced reader, for use when field is not visible
static class GetterMapping implements Advice.OffsetMapping {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public class EnhancerImpl implements Enhancer {

private static final CoreMessageLogger log = CoreLogging.messageLogger( Enhancer.class );
private static final Annotation HIBERNATE_VERSION_ANNOTATION;
private static final Annotation SUPPRESS_FB_WARNINGS_ANNOTATION;

static {
HIBERNATE_VERSION_ANNOTATION = new EnhancementInfo() {
Expand All @@ -84,6 +85,23 @@ public Class<? extends Annotation> annotationType() {
return EnhancementInfo.class;
}
};

SUPPRESS_FB_WARNINGS_ANNOTATION = new CodeTemplates.SuppressFBWarnings() {
@Override
public String[] value() {
return new String[0];
}

@Override
public String justification() {
return "generated code";
}

@Override
public Class<? extends Annotation> annotationType() {
return CodeTemplates.SuppressFBWarnings.class;
}
};
}

private static final AnnotationDescription TRANSIENT_ANNOTATION = AnnotationDescription.Builder.ofType( Transient.class ).build();
Expand Down Expand Up @@ -162,6 +180,16 @@ private TypePool buildTypePool(final ClassFileLocator classFileLocator) {
return TypePool.Default.WithLazyResolution.of( classFileLocator );
}

private List<Annotation> buildOptInAnnotationList(TypeDescription managedCtClass) {
List<Annotation> optInAnnotations = new ArrayList<>();

if ( enhancementContext.addSuppressFBWarnings( managedCtClass ) ) {
optInAnnotations.add(SUPPRESS_FB_WARNINGS_ANNOTATION);
}

return optInAnnotations;
}

private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDescription managedCtClass) {
// can't effectively enhance interfaces
if ( managedCtClass.isInterface() ) {
Expand All @@ -179,37 +207,43 @@ private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDes
return null;
}

List<Annotation> optInAnnotations = buildOptInAnnotationList( managedCtClass );

builder = builder.annotateType( HIBERNATE_VERSION_ANNOTATION );

if ( enhancementContext.isEntityClass( managedCtClass ) ) {
log.debugf( "Enhancing [%s] as Entity", managedCtClass.getName() );
builder = builder.implement( ManagedEntity.class )
.defineMethod( EnhancerConstants.ENTITY_INSTANCE_GETTER_NAME, Object.class, Visibility.PUBLIC )
.intercept( FixedValue.self() );
.intercept( FixedValue.self() )
.annotateMethod( optInAnnotations );

builder = addFieldWithGetterAndSetter(
builder,
EntityEntry.class,
optInAnnotations,
EnhancerConstants.ENTITY_ENTRY_FIELD_NAME,
EnhancerConstants.ENTITY_ENTRY_GETTER_NAME,
EnhancerConstants.ENTITY_ENTRY_SETTER_NAME
);
builder = addFieldWithGetterAndSetter(
builder,
ManagedEntity.class,
optInAnnotations,
EnhancerConstants.PREVIOUS_FIELD_NAME,
EnhancerConstants.PREVIOUS_GETTER_NAME,
EnhancerConstants.PREVIOUS_SETTER_NAME
);
builder = addFieldWithGetterAndSetter(
builder,
ManagedEntity.class,
optInAnnotations,
EnhancerConstants.NEXT_FIELD_NAME,
EnhancerConstants.NEXT_GETTER_NAME,
EnhancerConstants.NEXT_SETTER_NAME
);

builder = addInterceptorHandling( builder, managedCtClass );
builder = addInterceptorHandling( builder, managedCtClass, optInAnnotations );

if ( enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
List<AnnotatedFieldDescription> collectionFields = collectCollectionFields( managedCtClass );
Expand All @@ -218,42 +252,57 @@ private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDes
builder = builder.implement( SelfDirtinessTracker.class )
.defineField( EnhancerConstants.TRACKER_FIELD_NAME, DirtyTracker.class, FieldPersistence.TRANSIENT, Visibility.PRIVATE )
.annotateField( TRANSIENT_ANNOTATION )
.annotateField( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_CHANGER_NAME, void.class, Visibility.PUBLIC )
.withParameters( String.class )
.intercept( implementationTrackChange )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_GET_NAME, String[].class, Visibility.PUBLIC )
.intercept( implementationGetDirtyAttributesWithoutCollections )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_HAS_CHANGED_NAME, boolean.class, Visibility.PUBLIC )
.intercept( implementationAreFieldsDirtyWithoutCollections )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_CLEAR_NAME, void.class, Visibility.PUBLIC )
.intercept( implementationClearDirtyAttributesWithoutCollections )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_SUSPEND_NAME, void.class, Visibility.PUBLIC )
.withParameters( boolean.class )
.intercept( implementationSuspendDirtyTracking )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_COLLECTION_GET_NAME, CollectionTracker.class, Visibility.PUBLIC )
.intercept( implementationGetCollectionTrackerWithoutCollections );
.intercept( implementationGetCollectionTrackerWithoutCollections )
.annotateMethod( optInAnnotations );
}
else {
//TODO es.enableInterfaceExtendedSelfDirtinessTracker ? Careful with consequences..
builder = builder.implement( ExtendedSelfDirtinessTracker.class )
.defineField( EnhancerConstants.TRACKER_FIELD_NAME, DirtyTracker.class, FieldPersistence.TRANSIENT, Visibility.PRIVATE )
.annotateField( TRANSIENT_ANNOTATION )
.annotateField( optInAnnotations )
.defineField( EnhancerConstants.TRACKER_COLLECTION_NAME, CollectionTracker.class, FieldPersistence.TRANSIENT, Visibility.PRIVATE )
.annotateField( TRANSIENT_ANNOTATION )
.annotateField( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_CHANGER_NAME, void.class, Visibility.PUBLIC )
.withParameters( String.class )
.intercept( implementationTrackChange )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_GET_NAME, String[].class, Visibility.PUBLIC )
.intercept( implementationGetDirtyAttributes )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_HAS_CHANGED_NAME, boolean.class, Visibility.PUBLIC )
.intercept( implementationAreFieldsDirty )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_CLEAR_NAME, void.class, Visibility.PUBLIC )
.intercept( implementationClearDirtyAttributes )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_SUSPEND_NAME, void.class, Visibility.PUBLIC )
.withParameters( boolean.class )
.intercept( implementationSuspendDirtyTracking )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_COLLECTION_GET_NAME, CollectionTracker.class, Visibility.PUBLIC )
.intercept( FieldAccessor.ofField( EnhancerConstants.TRACKER_COLLECTION_NAME ) );
.intercept( FieldAccessor.ofField( EnhancerConstants.TRACKER_COLLECTION_NAME ) )
.annotateMethod( optInAnnotations );

Implementation isDirty = StubMethod.INSTANCE, getDirtyNames = StubMethod.INSTANCE, clearDirtyNames = StubMethod.INSTANCE;
for ( AnnotatedFieldDescription collectionField : collectionFields ) {
Expand Down Expand Up @@ -319,23 +368,26 @@ private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDes
.defineMethod( EnhancerConstants.TRACKER_COLLECTION_CHANGED_FIELD_NAME, void.class, Visibility.PUBLIC )
.withParameters( DirtyTracker.class )
.intercept( getDirtyNames )
.annotateMethod( optInAnnotations )
.defineMethod( EnhancerConstants.TRACKER_COLLECTION_CLEAR_NAME, void.class, Visibility.PUBLIC )
.intercept( Advice.withCustomMapping()
.to( CodeTemplates.ClearDirtyCollectionNames.class, adviceLocator )
.wrap( StubMethod.INSTANCE ) )
.annotateMethod( optInAnnotations )
.defineMethod( ExtendedSelfDirtinessTracker.REMOVE_DIRTY_FIELDS_NAME, void.class, Visibility.PUBLIC )
.withParameters( LazyAttributeLoadingInterceptor.class )
.intercept( clearDirtyNames );
.intercept( clearDirtyNames )
.annotateMethod( optInAnnotations );
}
}

return createTransformer( managedCtClass ).applyTo( builder );
return createTransformer( managedCtClass, optInAnnotations ).applyTo( builder );
}
else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
log.debugf( "Enhancing [%s] as Composite", managedCtClass.getName() );

builder = builder.implement( ManagedComposite.class );
builder = addInterceptorHandling( builder, managedCtClass );
builder = addInterceptorHandling( builder, managedCtClass, optInAnnotations);

if ( enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
builder = builder.implement( CompositeTracker.class )
Expand All @@ -346,42 +398,45 @@ else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
Visibility.PRIVATE
)
.annotateField( TRANSIENT_ANNOTATION )
.annotateField( optInAnnotations )
.defineMethod(
EnhancerConstants.TRACKER_COMPOSITE_SET_OWNER,
void.class,
Visibility.PUBLIC
)
.withParameters( String.class, CompositeOwner.class )
.intercept( implementationSetOwner )
.annotateMethod( optInAnnotations )
.defineMethod(
EnhancerConstants.TRACKER_COMPOSITE_CLEAR_OWNER,
void.class,
Visibility.PUBLIC
)
.withParameters( String.class )
.intercept( implementationClearOwner );
.intercept( implementationClearOwner )
.annotateMethod( optInAnnotations );
}

return createTransformer( managedCtClass ).applyTo( builder );
return createTransformer( managedCtClass, optInAnnotations).applyTo( builder );
}
else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) {
log.debugf( "Enhancing [%s] as MappedSuperclass", managedCtClass.getName() );

builder = builder.implement( ManagedMappedSuperclass.class );
return createTransformer( managedCtClass ).applyTo( builder );
return createTransformer( managedCtClass, optInAnnotations).applyTo( builder );
}
else if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
log.debugf( "Extended enhancement of [%s]", managedCtClass.getName() );
return createTransformer( managedCtClass ).applyExtended( builder );
return createTransformer( managedCtClass, optInAnnotations).applyExtended( builder );
}
else {
log.debugf( "Skipping enhancement of [%s]: not entity or composite", managedCtClass.getName() );
return null;
}
}

private PersistentAttributeTransformer createTransformer(TypeDescription typeDescription) {
return PersistentAttributeTransformer.collectPersistentFields( typeDescription, enhancementContext, typePool );
private PersistentAttributeTransformer createTransformer(TypeDescription typeDescription, List<Annotation> optInAnnotations) {
return PersistentAttributeTransformer.collectPersistentFields( typeDescription, optInAnnotations, enhancementContext, typePool );
}

// See HHH-10977 HHH-11284 HHH-11404 --- check for declaration of Managed interface on the class, not inherited
Expand All @@ -394,7 +449,7 @@ private boolean alreadyEnhanced(TypeDescription managedCtClass) {
return false;
}

private DynamicType.Builder<?> addInterceptorHandling(DynamicType.Builder<?> builder, TypeDescription managedCtClass) {
private DynamicType.Builder<?> addInterceptorHandling(DynamicType.Builder<?> builder, TypeDescription managedCtClass, List<Annotation> optInAnnotations) {
// interceptor handling is only needed if class has lazy-loadable attributes
if ( enhancementContext.hasLazyLoadableAttributes( managedCtClass ) ) {
log.debugf( "Weaving in PersistentAttributeInterceptable implementation on [%s]", managedCtClass.getName() );
Expand All @@ -404,6 +459,7 @@ private DynamicType.Builder<?> addInterceptorHandling(DynamicType.Builder<?> bui
builder = addFieldWithGetterAndSetter(
builder,
PersistentAttributeInterceptor.class,
optInAnnotations,
EnhancerConstants.INTERCEPTOR_FIELD_NAME,
EnhancerConstants.INTERCEPTOR_GETTER_NAME,
EnhancerConstants.INTERCEPTOR_SETTER_NAME
Expand All @@ -414,19 +470,23 @@ private DynamicType.Builder<?> addInterceptorHandling(DynamicType.Builder<?> bui
}

private static DynamicType.Builder<?> addFieldWithGetterAndSetter(
DynamicType.Builder<?> builder,
Class<?> type,
String fieldName,
String getterName,
String setterName) {
DynamicType.Builder<?> builder,
Class<?> type,
List<Annotation> annotations,
String fieldName,
String getterName,
String setterName) {
return builder
.defineField( fieldName, type, Visibility.PRIVATE, FieldPersistence.TRANSIENT )
.annotateField( TRANSIENT_ANNOTATION )
.annotateField( annotations )
.defineMethod( getterName, type, Visibility.PUBLIC )
.intercept( FieldAccessor.ofField( fieldName ) )
.annotateMethod( annotations )
.defineMethod( setterName, void.class, Visibility.PUBLIC )
.withParameters( type )
.intercept( FieldAccessor.ofField( fieldName ) );
.intercept( FieldAccessor.ofField( fieldName ) )
.annotateMethod( annotations );
}

private List<AnnotatedFieldDescription> collectCollectionFields(TypeDescription managedCtClass) {
Expand Down
Loading