Skip to content

Commit 43f3875

Browse files
franciscoovazevedofranciscoovazevedo
committed
fix: use default values for parameters in PMD configuration
1 parent 57e38da commit 43f3875

File tree

2 files changed

+247
-5
lines changed

2 files changed

+247
-5
lines changed

‎tools/pmdConfigCreator.go‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,18 @@ func generateRuleXML(rule Rule) (string, error) {
116116
// Generate rule with parameters
117117
var params strings.Builder
118118
for _, param := range rule.Parameters {
119-
if param.Name != "enabled" && param.Name != "version" { // Skip enabled and version parameters
119+
// Skip enabled, version, and parameters with empty values
120+
if param.Name != "enabled" && param.Name != "version" && param.Value != "" {
120121
params.WriteString(fmt.Sprintf(`
121122
<property name="%s" value="%s"/>`, param.Name, param.Value))
122123
}
123124
}
124125

126+
// If no parameters left after filtering, just output the rule without properties
127+
if params.Len() == 0 {
128+
return fmt.Sprintf(` <rule ref="%s"/>`, pmdRef), nil
129+
}
130+
125131
return fmt.Sprintf(` <rule ref="%s">
126132
<properties>%s
127133
</properties>
@@ -185,19 +191,51 @@ func CreatePmdConfig(configuration []domain.PatternConfiguration) string {
185191
patternEnabled := true
186192
var parameters []Parameter
187193

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)
188204
for _, param := range pattern.Parameters {
189205
if param.Name == "enabled" && param.Value == "false" {
190206
patternEnabled = false
191207
break
192208
} else if param.Name != "enabled" {
193209
// Store non-enabled parameters
194-
parameters = append(parameters, Parameter{
195-
Name: param.Name,
196-
Value: param.Value,
197-
})
210+
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]
215+
}
216+
217+
// Only add parameters with non-empty values
218+
if paramValue != "" {
219+
paramMap[param.Name] = paramValue
220+
}
221+
}
222+
}
223+
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
198228
}
199229
}
200230

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+
201239
// Apply prefix to pattern ID if needed
202240
patternID := pattern.PatternDefinition.Id
203241
if !strings.HasPrefix(patternID, "PMD_") && !strings.Contains(patternID, "/") {

‎tools/pmdConfigCreator_test.go‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,207 @@ func TestCreatePmdConfigEmpty(t *testing.T) {
179179
assert.Contains(t, obtainedConfig, `<rule ref="category/java/bestpractices.xml/AvoidReassigningParameters"/>`, "XML should contain expected rules")
180180
assert.Contains(t, obtainedConfig, `<rule ref="category/java/codestyle.xml/ClassNamingConventions">`, "XML should contain rules with properties")
181181
}
182+
183+
func TestCreatePmdConfigEmptyParameterValues(t *testing.T) {
184+
config := []domain.PatternConfiguration{
185+
{
186+
PatternDefinition: domain.PatternDefinition{
187+
Id: "java/codestyle/ClassNamingConventions",
188+
},
189+
Parameters: []domain.ParameterConfiguration{
190+
{
191+
Name: "enabled",
192+
Value: "true",
193+
},
194+
{
195+
Name: "testClassPattern",
196+
Value: "", // Empty value should be skipped
197+
},
198+
{
199+
Name: "abstractClassPattern",
200+
Value: "Abstract.*", // Non-empty value should be included
201+
},
202+
{
203+
Name: "classPattern",
204+
Value: "", // Empty value should be skipped
205+
},
206+
},
207+
},
208+
}
209+
210+
obtainedConfig := CreatePmdConfig(config)
211+
212+
var ruleset PMDRuleset
213+
err := xml.Unmarshal([]byte(obtainedConfig), &ruleset)
214+
if err != nil {
215+
t.Fatalf("Failed to parse generated XML: %v", err)
216+
}
217+
218+
// Find the ClassNamingConventions rule
219+
var classNamingRule *PMDRule
220+
for i, rule := range ruleset.Rules {
221+
if strings.Contains(rule.Ref, "ClassNamingConventions") {
222+
classNamingRule = &ruleset.Rules[i]
223+
break
224+
}
225+
}
226+
227+
// Rule should exist
228+
assert.NotNil(t, classNamingRule, "ClassNamingConventions rule should exist")
229+
230+
// Properties should exist
231+
assert.NotNil(t, classNamingRule.Properties, "Properties section should exist")
232+
233+
// Should only contain the non-empty parameter
234+
foundAbstractPattern := false
235+
for _, prop := range classNamingRule.Properties.Properties {
236+
// Should not find empty value parameters
237+
assert.NotEqual(t, "testClassPattern", prop.Name, "Empty parameter should be skipped")
238+
assert.NotEqual(t, "classPattern", prop.Name, "Empty parameter should be skipped")
239+
240+
// Should find non-empty parameter
241+
if prop.Name == "abstractClassPattern" {
242+
foundAbstractPattern = true
243+
assert.Equal(t, "Abstract.*", prop.Value)
244+
}
245+
}
246+
247+
assert.True(t, foundAbstractPattern, "Non-empty parameter should be included")
248+
}
249+
250+
func TestCreatePmdConfigWithDefaultParameterValues(t *testing.T) {
251+
config := []domain.PatternConfiguration{
252+
{
253+
PatternDefinition: domain.PatternDefinition{
254+
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+
},
260+
},
261+
},
262+
Parameters: []domain.ParameterConfiguration{
263+
{
264+
Name: "enabled",
265+
Value: "true",
266+
},
267+
{
268+
Name: "validTypes",
269+
Value: "", // Empty value should use default
270+
},
271+
},
272+
},
273+
}
274+
275+
obtainedConfig := CreatePmdConfig(config)
276+
277+
var ruleset PMDRuleset
278+
err := xml.Unmarshal([]byte(obtainedConfig), &ruleset)
279+
if err != nil {
280+
t.Fatalf("Failed to parse generated XML: %v", err)
281+
}
282+
283+
// Find the InvalidDependencyTypes rule
284+
var found bool
285+
for _, rule := range ruleset.Rules {
286+
if strings.Contains(rule.Ref, "InvalidDependencyTypes") {
287+
found = true
288+
// Rule should have properties
289+
assert.NotNil(t, rule.Properties, "Rule should have properties")
290+
291+
// Properties should contain validTypes with default value
292+
foundParam := false
293+
for _, prop := range rule.Properties.Properties {
294+
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")
297+
}
298+
}
299+
assert.True(t, foundParam, "validTypes parameter should be included with default value")
300+
}
301+
}
302+
assert.True(t, found, "InvalidDependencyTypes rule should exist")
303+
}
304+
305+
func TestDefaultParametersIncludedWhenNotSpecified(t *testing.T) {
306+
config := []domain.PatternConfiguration{
307+
{
308+
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+
},
336+
},
337+
Parameters: []domain.ParameterConfiguration{
338+
{
339+
Name: "enabled",
340+
Value: "true",
341+
},
342+
// No other parameters specified - should use defaults
343+
},
344+
},
345+
}
346+
347+
obtainedConfig := CreatePmdConfig(config)
348+
349+
var ruleset PMDRuleset
350+
err := xml.Unmarshal([]byte(obtainedConfig), &ruleset)
351+
if err != nil {
352+
t.Fatalf("Failed to parse generated XML: %v", err)
353+
}
354+
355+
// Find the AssignmentInOperand rule
356+
var found bool
357+
for _, rule := range ruleset.Rules {
358+
if strings.Contains(rule.Ref, "AssignmentInOperand") {
359+
found = true
360+
// Rule should have properties
361+
assert.NotNil(t, rule.Properties, "Rule should have properties")
362+
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+
}
372+
373+
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)
378+
}
379+
380+
// All expected parameters should have been found
381+
assert.Empty(t, expectedParams, "Not all default parameters were included")
382+
}
383+
}
384+
assert.True(t, found, "AssignmentInOperand rule should exist")
385+
}

0 commit comments

Comments
 (0)