From 9db451703ef0dbf4042fe0b7c341724e5637c4bc Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 06:12:40 +0000 Subject: [PATCH] fix: resolve Sonar findings (#279) Co-Authored-By: Andreas Igel --- .../roaster/RoasterCodeGenerator.java | 14 +-- .../ConfigurationProcessingTest.java | 98 ++++++++++--------- .../processor/testing/ProcessorTestUtils.java | 15 +-- 3 files changed, 68 insertions(+), 59 deletions(-) diff --git a/processor/src/main/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterCodeGenerator.java b/processor/src/main/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterCodeGenerator.java index 5df3f01e..a14e33ae 100644 --- a/processor/src/main/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterCodeGenerator.java +++ b/processor/src/main/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterCodeGenerator.java @@ -556,12 +556,14 @@ private void configureMethod( private void addParameter( MethodSource method, MethodParameterDto paramDto, boolean lastParameter) { - String parameterType = - lastParameter && paramDto.getParameterType() instanceof TypeNameArray arrayType - ? mapType(arrayType.getTypeOfArray()) - : mapType(paramDto.getParameterType()); - ParameterSource parameter = method.addParameter(parameterType, paramDto.getParameterName()); - if (lastParameter && paramDto.getParameterType() instanceof TypeNameArray) { + boolean varArgs = lastParameter && paramDto.getParameterType() instanceof TypeNameArray; + TypeName parameterType = + varArgs + ? ((TypeNameArray) paramDto.getParameterType()).getTypeOfArray() + : paramDto.getParameterType(); + ParameterSource parameter = + method.addParameter(mapType(parameterType), paramDto.getParameterName()); + if (varArgs) { parameter.setVarArgs(true); } applyAnnotations(parameter, paramDto.getAnnotations()); diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/ConfigurationProcessingTest.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/ConfigurationProcessingTest.java index cbebb097..7f61ecaa 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/ConfigurationProcessingTest.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/ConfigurationProcessingTest.java @@ -45,61 +45,17 @@ class ConfigurationProcessingTest { *
  • Add the parameter to BuilderConfiguration record *
  • Add builder methods in BuilderConfiguration.Builder *
  • Update DEFAULT configuration - *
  • Update this test to include the new option + *
  • Update these tests to include the new option * */ @Test void allConfigurationOptions_MustBeSettableViaBuilder() { - // This test will fail to compile if any builder method is missing - BuilderConfiguration config = - BuilderConfiguration.builder() - // Field setter generation options - .generateSupplier(OptionState.ENABLED) - .generateConsumer(OptionState.ENABLED) - .generateBuilderConsumer(OptionState.ENABLED) - // Conditional logic - .generateConditionalLogic(OptionState.ENABLED) - // Access control - .builderAccess(AccessModifier.PACKAGE_PRIVATE) - .builderConstructorAccess(AccessModifier.PRIVATE) - .methodAccess(AccessModifier.PACKAGE_PRIVATE) - // Helper method generation - .generateVarArgsHelpers(OptionState.ENABLED) - .generateStringFormatHelpers(OptionState.ENABLED) - .generateUnboxedOptional(OptionState.ENABLED) - .copyTypeAnnotations(OptionState.ENABLED) - // Collection builder options - .usingArrayListBuilder(OptionState.ENABLED) - .usingArrayListBuilderWithElementBuilders(OptionState.ENABLED) - .usingHashSetBuilder(OptionState.ENABLED) - .usingHashSetBuilderWithElementBuilders(OptionState.ENABLED) - .usingHashMapBuilder(OptionState.ENABLED) - // Annotations - .usingGeneratedAnnotation(OptionState.ENABLED) - .usingBuilderImplementationAnnotation(OptionState.ENABLED) - // Integration - .implementsBuilderBase(OptionState.ENABLED) - .generateWithInterface(OptionState.ENABLED) - .usingJacksonDeserializerAnnotation(OptionState.ENABLED) - .generateJacksonModule(OptionState.ENABLED) - // Documentation - .generateJavaDoc(OptionState.ENABLED) - // Naming - .builderSuffix("Builder") - .setterSuffix("") - // Formatting - .formattingMode("lightweight") - .build(); - - // Verify all options are accessible (this will fail to compile if accessors are missing) + BuilderConfiguration config = buildFullyConfigured(); assertNotNull(config); assertEquals(OptionState.ENABLED, config.generateFieldSupplier()); assertEquals(OptionState.ENABLED, config.generateFieldConsumer()); assertEquals(OptionState.ENABLED, config.generateBuilderConsumer()); assertEquals(OptionState.ENABLED, config.generateConditionalHelper()); - assertEquals(AccessModifier.PACKAGE_PRIVATE, config.getBuilderAccess()); - assertEquals(AccessModifier.PRIVATE, config.getBuilderConstructorAccess()); - assertEquals(AccessModifier.PACKAGE_PRIVATE, config.getMethodAccess()); assertEquals(OptionState.ENABLED, config.generateVarArgsHelpers()); assertEquals(OptionState.ENABLED, config.generateStringFormatHelpers()); assertEquals(OptionState.ENABLED, config.generateUnboxedOptional()); @@ -116,11 +72,61 @@ void allConfigurationOptions_MustBeSettableViaBuilder() { assertEquals(OptionState.ENABLED, config.usingJacksonDeserializerAnnotation()); assertEquals(OptionState.ENABLED, config.generateJacksonModule()); assertEquals(OptionState.ENABLED, config.generateJavaDoc()); + } + + @Test + void allConfigurationOptions_AccessNamingAndFormatting_MustBeReadable() { + BuilderConfiguration config = buildFullyConfigured(); + + assertEquals(AccessModifier.PACKAGE_PRIVATE, config.getBuilderAccess()); + assertEquals(AccessModifier.PRIVATE, config.getBuilderConstructorAccess()); + assertEquals(AccessModifier.PACKAGE_PRIVATE, config.getMethodAccess()); assertEquals("Builder", config.getBuilderSuffix()); assertEquals("", config.getSetterSuffix()); assertEquals("lightweight", config.formattingMode()); } + private static BuilderConfiguration buildFullyConfigured() { + return BuilderConfiguration.builder() + // Field setter generation options + .generateSupplier(OptionState.ENABLED) + .generateConsumer(OptionState.ENABLED) + .generateBuilderConsumer(OptionState.ENABLED) + // Conditional logic + .generateConditionalLogic(OptionState.ENABLED) + // Access control + .builderAccess(AccessModifier.PACKAGE_PRIVATE) + .builderConstructorAccess(AccessModifier.PRIVATE) + .methodAccess(AccessModifier.PACKAGE_PRIVATE) + // Helper method generation + .generateVarArgsHelpers(OptionState.ENABLED) + .generateStringFormatHelpers(OptionState.ENABLED) + .generateUnboxedOptional(OptionState.ENABLED) + .copyTypeAnnotations(OptionState.ENABLED) + // Collection builder options + .usingArrayListBuilder(OptionState.ENABLED) + .usingArrayListBuilderWithElementBuilders(OptionState.ENABLED) + .usingHashSetBuilder(OptionState.ENABLED) + .usingHashSetBuilderWithElementBuilders(OptionState.ENABLED) + .usingHashMapBuilder(OptionState.ENABLED) + // Annotations + .usingGeneratedAnnotation(OptionState.ENABLED) + .usingBuilderImplementationAnnotation(OptionState.ENABLED) + // Integration + .implementsBuilderBase(OptionState.ENABLED) + .generateWithInterface(OptionState.ENABLED) + .usingJacksonDeserializerAnnotation(OptionState.ENABLED) + .generateJacksonModule(OptionState.ENABLED) + // Documentation + .generateJavaDoc(OptionState.ENABLED) + // Naming + .builderSuffix("Builder") + .setterSuffix("") + // Formatting + .formattingMode("lightweight") + .build(); + } + /** * Compiler arguments integration test: Verify generated builder with all options disabled. * diff --git a/processor/src/test/java/org/javahelpers/simple/builders/processor/testing/ProcessorTestUtils.java b/processor/src/test/java/org/javahelpers/simple/builders/processor/testing/ProcessorTestUtils.java index d7a70427..7b67fa84 100644 --- a/processor/src/test/java/org/javahelpers/simple/builders/processor/testing/ProcessorTestUtils.java +++ b/processor/src/test/java/org/javahelpers/simple/builders/processor/testing/ProcessorTestUtils.java @@ -16,6 +16,12 @@ */ public final class ProcessorTestUtils { + private static final Pattern PACKAGE_PATTERN = + Pattern.compile("(?m)^\\s*package\\s+([a-zA-Z_]\\w*(?:\\.[a-zA-Z_]\\w*)*)\\s*;"); + private static final Pattern TOP_LEVEL_TYPE_PATTERN = + Pattern.compile( + "(?m)^[ \\t]*(?:(?:public|protected|private|abstract|final|static|sealed|non-sealed|strictfp)\\s+)*(?:@?interface|class|enum|record)\\s+([A-Za-z_]\\w*)\\b"); + private ProcessorTestUtils() {} /** @@ -184,17 +190,12 @@ public static JavaFileObject forSource(String source) { } private static String extractPackageName(String source) { - Matcher m = - Pattern.compile("(?m)^\\s*package\\s+([a-zA-Z_]\\w*(?:\\.[a-zA-Z_]\\w*)*)\\s*;") - .matcher(source); + Matcher m = PACKAGE_PATTERN.matcher(source); return m.find() ? m.group(1) : null; } private static String extractTopLevelTypeName(String source) { - Matcher m = - Pattern.compile( - "(?m)^\\s*(?:public|protected|private)?(?:\\s+(?:abstract|final|static|sealed|non-sealed|strictfp))*\\s*(?:@?interface|class|enum|record)\\s+([A-Za-z_]\\w*)\\b") - .matcher(source); + Matcher m = TOP_LEVEL_TYPE_PATTERN.matcher(source); return m.find() ? m.group(1) : null; }