Skip to content

Commit 3b65b8a

Browse files
franciscoovazevedofranciscoovazevedo
committed
fix: correct PMD parameter value parsing
1 parent 43f3875 commit 3b65b8a

File tree

2 files changed

+90
-113
lines changed

2 files changed

+90
-113
lines changed

‎tools/pmdConfigCreator.go‎

Lines changed: 16 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ func generateRuleXML(rule Rule) (string, error) {
116116
// Generate rule with parameters
117117
var params strings.Builder
118118
for _, param := range rule.Parameters {
119-
// Skip enabled, version, and parameters with empty values
120-
if param.Name != "enabled" && param.Name != "version" && param.Value != "" {
119+
// Skip enabled and version parameters, but include all others
120+
if param.Name != "enabled" && param.Name != "version" {
121121
params.WriteString(fmt.Sprintf(`
122122
<property name="%s" value="%s"/>`, param.Name, param.Value))
123123
}
@@ -128,10 +128,12 @@ func generateRuleXML(rule Rule) (string, error) {
128128
return fmt.Sprintf(` <rule ref="%s"/>`, pmdRef), nil
129129
}
130130

131-
return fmt.Sprintf(` <rule ref="%s">
131+
result := fmt.Sprintf(` <rule ref="%s">
132132
<properties>%s
133133
</properties>
134-
</rule>`, pmdRef, params.String()), nil
134+
</rule>`, pmdRef, params.String())
135+
136+
return result, nil
135137
}
136138

137139
// ConvertToPMDRuleset converts Codacy rules to PMD ruleset format
@@ -191,51 +193,28 @@ func CreatePmdConfig(configuration []domain.PatternConfiguration) string {
191193
patternEnabled := true
192194
var parameters []Parameter
193195

194-
// Create a map of default values from pattern definition
195-
defaultValues := make(map[string]string)
196-
for _, defParam := range pattern.PatternDefinition.Parameters {
197-
if defParam.Default != "" {
198-
defaultValues[defParam.Name] = defParam.Default
199-
}
200-
}
201-
202-
// Process user-defined parameters first
203-
paramMap := make(map[string]string)
196+
// Process user-defined parameters
204197
for _, param := range pattern.Parameters {
205198
if param.Name == "enabled" && param.Value == "false" {
206199
patternEnabled = false
207200
break
208-
} else if param.Name != "enabled" {
209-
// Store non-enabled parameters
201+
} else if param.Name != "enabled" && param.Name != "version" {
202+
// Check for a value - use Value if not empty, otherwise use Default
210203
paramValue := param.Value
211-
212-
// If value is empty but we have a default, use the default
213-
if paramValue == "" && defaultValues[param.Name] != "" {
214-
paramValue = defaultValues[param.Name]
204+
if paramValue == "" && param.Default != "" {
205+
paramValue = param.Default
215206
}
216207

217-
// Only add parameters with non-empty values
208+
// Add parameter if it has a value
218209
if paramValue != "" {
219-
paramMap[param.Name] = paramValue
210+
parameters = append(parameters, Parameter{
211+
Name: param.Name,
212+
Value: paramValue,
213+
})
220214
}
221215
}
222216
}
223217

224-
// Also include default parameters that weren't specified in user parameters
225-
for name, value := range defaultValues {
226-
if _, exists := paramMap[name]; !exists {
227-
paramMap[name] = value
228-
}
229-
}
230-
231-
// Convert parameter map to slice
232-
for name, value := range paramMap {
233-
parameters = append(parameters, Parameter{
234-
Name: name,
235-
Value: value,
236-
})
237-
}
238-
239218
// Apply prefix to pattern ID if needed
240219
patternID := pattern.PatternDefinition.Id
241220
if !strings.HasPrefix(patternID, "PMD_") && !strings.Contains(patternID, "/") {

‎tools/pmdConfigCreator_test.go‎

Lines changed: 74 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package tools
22

33
import (
44
"encoding/xml"
5+
"fmt"
56
"os"
67
"path/filepath"
78
"strings"
@@ -247,17 +248,37 @@ func TestCreatePmdConfigEmptyParameterValues(t *testing.T) {
247248
assert.True(t, foundAbstractPattern, "Non-empty parameter should be included")
248249
}
249250

250-
func TestCreatePmdConfigWithDefaultParameterValues(t *testing.T) {
251+
func TestEmptyParametersAreSkipped(t *testing.T) {
251252
config := []domain.PatternConfiguration{
252253
{
253254
PatternDefinition: domain.PatternDefinition{
254255
Id: "PMD_category_pom_errorprone_InvalidDependencyTypes",
255-
Parameters: []domain.ParameterConfiguration{
256-
{
257-
Name: "validTypes",
258-
Default: "pom,jar,maven-plugin,ejb,war,ear,rar,par",
259-
},
256+
},
257+
Parameters: []domain.ParameterConfiguration{
258+
{
259+
Name: "enabled",
260+
Value: "true",
260261
},
262+
{
263+
Name: "validTypes",
264+
Value: "", // Empty value - should produce simple rule reference
265+
},
266+
},
267+
},
268+
}
269+
270+
obtainedConfig := CreatePmdConfig(config)
271+
272+
// Should find this exact pattern in the generated XML
273+
expectedPattern := `<rule ref="category/pom/errorprone.xml/InvalidDependencyTypes"/>`
274+
assert.Contains(t, obtainedConfig, expectedPattern, "XML should contain the simple rule reference without properties")
275+
}
276+
277+
func TestNonEmptyParameterValue(t *testing.T) {
278+
config := []domain.PatternConfiguration{
279+
{
280+
PatternDefinition: domain.PatternDefinition{
281+
Id: "PMD_category_pom_errorprone_InvalidDependencyTypes",
261282
},
262283
Parameters: []domain.ParameterConfiguration{
263284
{
@@ -266,120 +287,97 @@ func TestCreatePmdConfigWithDefaultParameterValues(t *testing.T) {
266287
},
267288
{
268289
Name: "validTypes",
269-
Value: "", // Empty value should use default
290+
Value: "pom,jar,maven-plugin,ejb,war,ear,rar,par", // Non-empty value should be used
270291
},
271292
},
272293
},
273294
}
274295

275296
obtainedConfig := CreatePmdConfig(config)
276297

298+
// Debug output
299+
fmt.Printf("Generated XML:\n%s\n", obtainedConfig)
300+
277301
var ruleset PMDRuleset
278302
err := xml.Unmarshal([]byte(obtainedConfig), &ruleset)
279-
if err != nil {
280-
t.Fatalf("Failed to parse generated XML: %v", err)
281-
}
303+
assert.NoError(t, err)
282304

283-
// Find the InvalidDependencyTypes rule
284-
var found bool
305+
// Find our rule
306+
found := false
285307
for _, rule := range ruleset.Rules {
286-
if strings.Contains(rule.Ref, "InvalidDependencyTypes") {
308+
if rule.Ref == "category/pom/errorprone.xml/InvalidDependencyTypes" {
287309
found = true
288-
// Rule should have properties
310+
// Check properties
289311
assert.NotNil(t, rule.Properties, "Rule should have properties")
312+
assert.NotEmpty(t, rule.Properties.Properties, "Rule should have property values")
290313

291-
// Properties should contain validTypes with default value
292-
foundParam := false
314+
// Check for validTypes property
315+
foundValidTypes := false
293316
for _, prop := range rule.Properties.Properties {
294317
if prop.Name == "validTypes" {
295-
foundParam = true
296-
assert.Equal(t, "pom,jar,maven-plugin,ejb,war,ear,rar,par", prop.Value, "Property should use default value")
318+
foundValidTypes = true
319+
assert.Equal(t, "pom,jar,maven-plugin,ejb,war,ear,rar,par", prop.Value,
320+
"Should use the non-empty value from parameters")
297321
}
298322
}
299-
assert.True(t, foundParam, "validTypes parameter should be included with default value")
323+
assert.True(t, foundValidTypes, "validTypes property should be present")
300324
}
301325
}
302-
assert.True(t, found, "InvalidDependencyTypes rule should exist")
326+
assert.True(t, found, "Rule should be present")
303327
}
304328

305-
func TestDefaultParametersIncludedWhenNotSpecified(t *testing.T) {
329+
func TestExactJsonStructure(t *testing.T) {
330+
// This test exactly matches the JSON structure from the API
306331
config := []domain.PatternConfiguration{
307332
{
308333
PatternDefinition: domain.PatternDefinition{
309-
Id: "PMD_category_ecmascript_codestyle_AssignmentInOperand",
310-
Parameters: []domain.ParameterConfiguration{
311-
{
312-
Name: "allowWhile",
313-
Default: "false",
314-
},
315-
{
316-
Name: "allowIf",
317-
Default: "false",
318-
},
319-
{
320-
Name: "allowTernaryResults",
321-
Default: "false",
322-
},
323-
{
324-
Name: "allowTernary",
325-
Default: "false",
326-
},
327-
{
328-
Name: "allowFor",
329-
Default: "false",
330-
},
331-
{
332-
Name: "allowIncrementDecrement",
333-
Default: "false",
334-
},
335-
},
334+
Id: "PMD_category_pom_errorprone_InvalidDependencyTypes",
336335
},
337336
Parameters: []domain.ParameterConfiguration{
338337
{
339-
Name: "enabled",
340-
Value: "true",
338+
Name: "validTypes",
339+
Value: "pom,jar,maven-plugin,ejb,war,ear,rar,par", // This value should be preserved
341340
},
342-
// No other parameters specified - should use defaults
343341
},
344342
},
345343
}
346344

345+
fmt.Println("Test input:")
346+
fmt.Printf("Parameters: %+v\n", config[0].Parameters)
347+
347348
obtainedConfig := CreatePmdConfig(config)
349+
fmt.Println("Generated XML:")
350+
fmt.Println(obtainedConfig)
348351

349352
var ruleset PMDRuleset
350353
err := xml.Unmarshal([]byte(obtainedConfig), &ruleset)
351-
if err != nil {
352-
t.Fatalf("Failed to parse generated XML: %v", err)
353-
}
354+
assert.NoError(t, err)
354355

355-
// Find the AssignmentInOperand rule
356-
var found bool
356+
// Find the rule
357+
found := false
357358
for _, rule := range ruleset.Rules {
358-
if strings.Contains(rule.Ref, "AssignmentInOperand") {
359+
if rule.Ref == "category/pom/errorprone.xml/InvalidDependencyTypes" {
359360
found = true
360-
// Rule should have properties
361-
assert.NotNil(t, rule.Properties, "Rule should have properties")
362361

363-
// Check all the default parameters are included
364-
expectedParams := map[string]string{
365-
"allowWhile": "false",
366-
"allowIf": "false",
367-
"allowTernaryResults": "false",
368-
"allowTernary": "false",
369-
"allowFor": "false",
370-
"allowIncrementDecrement": "false",
371-
}
362+
// Properties section should exist
363+
assert.NotNil(t, rule.Properties, "Properties section should exist")
372364

365+
// Should have the validTypes property with the correct value
366+
validTypesFound := false
373367
for _, prop := range rule.Properties.Properties {
374-
expectedValue, exists := expectedParams[prop.Name]
375-
assert.True(t, exists, "Unexpected parameter: %s", prop.Name)
376-
assert.Equal(t, expectedValue, prop.Value, "Parameter %s has wrong value", prop.Name)
377-
delete(expectedParams, prop.Name)
368+
if prop.Name == "validTypes" {
369+
validTypesFound = true
370+
assert.Equal(t, "pom,jar,maven-plugin,ejb,war,ear,rar,par", prop.Value,
371+
"Value should be preserved exactly")
372+
}
378373
}
379-
380-
// All expected parameters should have been found
381-
assert.Empty(t, expectedParams, "Not all default parameters were included")
374+
assert.True(t, validTypesFound, "validTypes property should be present")
382375
}
383376
}
384-
assert.True(t, found, "AssignmentInOperand rule should exist")
377+
assert.True(t, found, "Rule should be present")
378+
}
379+
380+
func TestParameterWithDefaultFieldInsteadOfValue(t *testing.T) {
381+
// Renaming this test since it's not about default fields anymore
382+
t.Skip("Skipping test related to default values")
385383
}

0 commit comments

Comments
 (0)