feat: upgrade converter with validation by sehwan505 · Pull Request #30 · DevSymphony/sym-cli

sehwan505 · 2025-11-26T03:00:41Z

upgrade converter with validation

Copilot

Pull request overview

This PR enhances the converter with validation capabilities to ensure LLM-generated linter configurations use only valid, native rules and options. It introduces registries for ESLint rules, Prettier options, and TypeScript compiler options, validates LLM outputs against these registries, and improves prompts to guide the LLM toward valid configurations.

Key Changes

New validation registry: Added registry.go with comprehensive registries of valid ESLint rules (60+ rules), Prettier options (14 options), and TypeScript compiler options (20+ options) along with their schemas
Validation integration: Integrated validation into the ESLint, Prettier, and TSC converters to filter out invalid LLM-generated options
Enhanced LLM prompts: Updated system prompts to explicitly list valid options and provide clearer guidance on when to return empty configurations

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.

File	Description
`internal/converter/linters/registry.go`	New file defining validation registries and functions for ESLint rules, Prettier options, and TSC options with comprehensive rule definitions and validation logic
`internal/converter/linters/prettier_tsc.go`	Integrated validation for Prettier and TSC options, updated prompts to include valid option lists, and added filtering logic to remove invalid options
`internal/converter/linters/eslint.go`	Added ESLint rule validation, enhanced prompts with complete rule list, and integrated validation to reject invalid rules
`internal/converter/converter.go`	Updated documentation to expand checkstyle capabilities description with more specific rule names

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copilot · 2025-11-26T03:05:53Z

internal/converter/linters/eslint.go

+		fmt.Printf("⚠️  Invalid ESLint rule '%s': %s\n", result.RuleName, validation.Message)
+		return "", nil, nil


Using fmt.Printf to output warnings directly to stdout is not ideal for library code. Consider one of these alternatives:

Use a logger interface that can be injected (e.g., log.Printf or a structured logger)

Collect warnings in the result structure and return them

Return warnings through the error return value using a custom error type that includes warnings

This would make the code more testable and allow callers to control how warnings are handled.

Copilot · 2025-11-26T03:05:53Z

internal/converter/linters/registry.go

+		}
+	}
+
+	// TODO: Add options validation based on OptionsSchema


The TODO comment indicates that options validation is not yet implemented. This means the validation only checks if a rule name exists, but not whether the provided options match the rule's expected schema. Consider implementing this validation to ensure the LLM-generated options conform to the rule's schema definition, or create a tracking issue if this is intended for future work.

Suggested change

// TODO: Add options validation based on OptionsSchema

// Validate options against OptionsSchema

schema := def.Options

// If no options schema, accept any options

if schema.Type == "" {

return ValidationError{Valid: true}

}

// If schema expects an object, options should be a map[string]interface{}

if schema.Type == "object" {

optsMap, ok := options.(map[string]interface{})

if !ok {

return ValidationError{

Valid: false,

Message: "options should be an object (map)",

Suggestion: "Provide options as a key-value map.",

}

}

for propName, propSchema := range schema.Properties {

val, exists := optsMap[propName]

if !exists {

continue // Option is optional if not present

}

// Type check

switch propSchema.Type {

case "string":

_, ok := val.(string)

if !ok {

return ValidationError{

Valid: false,

Message: "option '" + propName + "' should be a string",

Suggestion: "Provide a string value for '" + propName + "'.",

}

}

// Enum check

if len(propSchema.Enum) > 0 {

strVal := val.(string)

valid := false

for _, allowed := range propSchema.Enum {

if strVal == allowed {

valid = true

break

}

}

if !valid {

return ValidationError{

Valid: false,

Message: "invalid value for '" + propName + "'",

Suggestion: "Valid values: " + joinStrings(propSchema.Enum),

}

}

}

case "array":

arr, ok := val.([]interface{})

if !ok {

return ValidationError{

Valid: false,

Message: "option '" + propName + "' should be an array",

Suggestion: "Provide an array value for '" + propName + "'.",

}

}

// Check item type

for _, item := range arr {

switch propSchema.Items {

case "string":

if _, ok := item.(string); !ok {

return ValidationError{

Valid: false,

Message: "array item in '" + propName + "' should be a string",

Suggestion: "All items in '" + propName + "' should be strings.",

}

}

// Add other types as needed

}

}

case "number":

_, ok := val.(float64)

if !ok {

return ValidationError{

Valid: false,

Message: "option '" + propName + "' should be a number",

Suggestion: "Provide a numeric value for '" + propName + "'.",

}

}

case "boolean":

_, ok := val.(bool)

if !ok {

return ValidationError{

Valid: false,

Message: "option '" + propName + "' should be a boolean",

Suggestion: "Provide a boolean value for '" + propName + "'.",

}

}

// Add other types as needed

}

}

}

// Add support for other schema types if needed

Copilot · 2025-11-26T03:05:53Z

internal/converter/converter.go

+- checkstyle: Java naming and style checks ONLY (ClassName, MethodName, VariableName, ConstantName, ParameterName, LocalVariableName, LineLength, MethodLength, 
+ParameterNumber, FileLength, Indentation, WhitespaceAround, NeedBraces, LeftCurly, RightCurly, AvoidStarImport, 
+IllegalImport, UnusedImports, CyclomaticComplexity, NPathComplexity, JavadocMethod, JavadocType, MissingJavadocMethod)


[nitpick] These lines are very long (over 160 characters) which may affect readability and could exceed style guide limits. Consider breaking the checkstyle capabilities list into multiple lines for better readability:

- checkstyle: Java naming and style checks ONLY (ClassName, MethodName, VariableName, ConstantName, ParameterName, LocalVariableName, LineLength, MethodLength, ParameterNumber, FileLength, Indentation, WhitespaceAround, NeedBraces, LeftCurly, RightCurly, AvoidStarImport, IllegalImport, UnusedImports, CyclomaticComplexity, NPathComplexity, JavadocMethod, JavadocType, MissingJavadocMethod)

Suggested change

- checkstyle: Java naming and style checks ONLY (ClassName, MethodName, VariableName, ConstantName, ParameterName, LocalVariableName, LineLength, MethodLength,

ParameterNumber, FileLength, Indentation, WhitespaceAround, NeedBraces, LeftCurly, RightCurly, AvoidStarImport,

IllegalImport, UnusedImports, CyclomaticComplexity, NPathComplexity, JavadocMethod, JavadocType, MissingJavadocMethod)

- checkstyle: Java naming and style checks ONLY (

ClassName, MethodName, VariableName, ConstantName, ParameterName, LocalVariableName, LineLength, MethodLength,

ParameterNumber, FileLength, Indentation, WhitespaceAround, NeedBraces, LeftCurly, RightCurly, AvoidStarImport,

IllegalImport, UnusedImports, CyclomaticComplexity, NPathComplexity, JavadocMethod, JavadocType, MissingJavadocMethod

)

Copilot · 2025-11-26T03:05:54Z

internal/converter/linters/prettier_tsc.go

+	// VALIDATION: Filter out invalid options
+	validConfig := make(map[string]interface{})
+	for key, value := range config {
+		validation := ValidatePrettierOption(key, value)
+		if validation.Valid {
+			validConfig[key] = value
+		} else {
+			fmt.Printf("⚠️  Invalid Prettier option '%s': %s\n", key, validation.Message)
+		}
+	}
+
+	return validConfig, nil


The new validation logic that filters out invalid Prettier options lacks test coverage. Consider adding tests to verify:

Valid options are kept in the result

Invalid options are filtered out

The warning message is logged (or consider returning warnings in a testable way)

Empty configuration is handled correctly when all options are invalid

Copilot · 2025-11-26T03:05:54Z

internal/converter/linters/prettier_tsc.go

+	// VALIDATION: Filter out invalid options
+	validConfig := make(map[string]interface{})
+	for key, value := range config {
+		validation := ValidateTSCOption(key, value)
+		if validation.Valid {
+			validConfig[key] = value
+		} else {
+			fmt.Printf("⚠️  Invalid TSC option '%s': %s\n", key, validation.Message)
+		}
+	}
+
+	return validConfig, nil


The new validation logic that filters out invalid TSC options lacks test coverage. Consider adding tests to verify:

Valid options are kept in the result

Invalid options are filtered out

The warning message is logged (or consider returning warnings in a testable way)

Empty configuration is handled correctly when all options are invalid

Copilot · 2025-11-26T03:05:54Z

internal/converter/linters/prettier_tsc.go

+			fmt.Printf("⚠️  Invalid TSC option '%s': %s\n", key, validation.Message)
+		}


Using fmt.Printf to output warnings directly to stdout is not ideal for library code. Consider one of these alternatives:

Use a logger interface that can be injected (e.g., log.Printf or a structured logger)

Collect warnings in the result structure and return them

Return warnings through the error return value using a custom error type that includes warnings

This would make the code more testable and allow callers to control how warnings are handled.

Copilot · 2025-11-26T03:05:55Z

internal/converter/linters/registry.go

+	// Validate type and enum if applicable
+	if len(def.Enum) > 0 {
+		strVal, ok := value.(string)
+		if ok {
+			valid := false
+			for _, allowed := range def.Enum {
+				if strVal == allowed {
+					valid = true
+					break
+				}
+			}
+			if !valid {
+				return ValidationError{
+					Valid:      false,
+					Message:    "invalid value for " + optionName,
+					Suggestion: "Valid values: " + joinStrings(def.Enum),
+				}
+			}
+		}
+	}


The validation only checks enum values when the value is already a string (if ok), but doesn't validate when:

The value type doesn't match the expected type (e.g., a number when a string is expected)

The value is not a string but enum validation is required

Consider adding type validation before enum validation. For example:

// Validate type switch def.Type { case "string": if _, ok := value.(string); !ok { return ValidationError{Valid: false, Message: "expected string value for " + optionName} } case "number": if _, ok := value.(float64); !ok { return ValidationError{Valid: false, Message: "expected number value for " + optionName} } case "boolean": if _, ok := value.(bool); !ok { return ValidationError{Valid: false, Message: "expected boolean value for " + optionName} } }

Copilot · 2025-11-26T03:05:55Z

internal/converter/linters/registry.go

+			Message: "unknown TypeScript compiler option: " + optionName,
+		}
+	}
+


The ValidateTSCOption function only checks if the option exists but doesn't validate:

The value type (all TSC options in the registry are booleans with defaults, but no type checking is performed)

The value is appropriate for the option

Consider adding type validation similar to ValidatePrettierOption to ensure boolean values are actually booleans. For example:

def, exists := TSCOptionRegistry[optionName] if !exists { return ValidationError{Valid: false, Message: "unknown TypeScript compiler option: " + optionName} } // Validate type (all TSC options are boolean) if _, ok := value.(bool); !ok { return ValidationError{Valid: false, Message: "expected boolean value for " + optionName} }

Suggested change

// Validate type (all TSC options are boolean)

if _, ok := value.(bool); !ok {

return ValidationError{

Valid: false,

Message: "expected boolean value for " + optionName,

}

}

Copilot · 2025-11-26T03:05:55Z

internal/converter/linters/registry.go

+// ValidateESLintRule checks if a rule name and options are valid
+func ValidateESLintRule(ruleName string, options interface{}) ValidationError {
+	def, exists := ESLintRuleRegistry[ruleName]
+	if !exists {
+		return ValidationError{
+			Valid:   false,
+			Message: "unknown ESLint rule: " + ruleName,
+			Suggestion: "This rule may require a plugin or doesn't exist. " +
+				"Consider using llm-validator for this check instead.",
+		}
+	}
+
+	if def.Deprecated {
+		return ValidationError{
+			Valid:      false,
+			Message:    "deprecated rule: " + ruleName,
+			Suggestion: "Use '" + def.Replacement + "' instead.",
+		}
+	}
+
+	// TODO: Add options validation based on OptionsSchema
+	return ValidationError{Valid: true}
+}
+
+// ValidatePrettierOption checks if a Prettier option is valid
+func ValidatePrettierOption(optionName string, value interface{}) ValidationError {
+	def, exists := PrettierOptionRegistry[optionName]
+	if !exists {
+		return ValidationError{
+			Valid:   false,
+			Message: "unknown Prettier option: " + optionName,
+		}
+	}
+
+	// Validate type and enum if applicable
+	if len(def.Enum) > 0 {
+		strVal, ok := value.(string)
+		if ok {
+			valid := false
+			for _, allowed := range def.Enum {
+				if strVal == allowed {
+					valid = true
+					break
+				}
+			}
+			if !valid {
+				return ValidationError{
+					Valid:      false,
+					Message:    "invalid value for " + optionName,
+					Suggestion: "Valid values: " + joinStrings(def.Enum),
+				}
+			}
+		}
+	}
+
+	return ValidationError{Valid: true}
+}
+
+// ValidateTSCOption checks if a TypeScript compiler option is valid
+func ValidateTSCOption(optionName string, value interface{}) ValidationError {
+	_, exists := TSCOptionRegistry[optionName]
+	if !exists {
+		return ValidationError{
+			Valid:   false,
+			Message: "unknown TypeScript compiler option: " + optionName,
+		}
+	}
+
+	return ValidationError{Valid: true}
+}
+
+// ValidationError represents a validation result
+type ValidationError struct {
+	Valid      bool
+	Message    string
+	Suggestion string
+}
+
+func joinStrings(strs []string) string {
+	result := ""
+	for i, s := range strs {
+		if i > 0 {
+			result += ", "
+		}
+		result += s
+	}
+	return result
+}
+
+// GetESLintRuleNames returns all valid ESLint rule names
+func GetESLintRuleNames() []string {
+	names := make([]string, 0, len(ESLintRuleRegistry))
+	for name := range ESLintRuleRegistry {
+		names = append(names, name)
+	}
+	return names
+}
+
+// GetPrettierOptionNames returns all valid Prettier option names
+func GetPrettierOptionNames() []string {
+	names := make([]string, 0, len(PrettierOptionRegistry))
+	for name := range PrettierOptionRegistry {
+		names = append(names, name)
+	}
+	return names
+}
+
+// GetTSCOptionNames returns all valid TypeScript compiler option names
+func GetTSCOptionNames() []string {
+	names := make([]string, 0, len(TSCOptionRegistry))
+	for name := range TSCOptionRegistry {
+		names = append(names, name)
+	}
+	return names
+}


The new validation functions (ValidateESLintRule, ValidatePrettierOption, ValidateTSCOption) and registry helper functions (GetESLintRuleNames, GetPrettierOptionNames, GetTSCOptionNames) lack test coverage. Given that the repository has comprehensive tests in the tests/ directory, consider adding unit tests to verify:

Valid rule/option names are accepted

Invalid rule/option names are rejected with appropriate error messages

Deprecated rules are detected (for ESLint)

Enum validation works correctly (for Prettier)

Type validation works as expected

Edge cases (empty strings, nil values, etc.)

Copilot · 2025-11-26T03:05:55Z

internal/converter/linters/prettier_tsc.go

+			fmt.Printf("⚠️  Invalid Prettier option '%s': %s\n", key, validation.Message)
+		}


Using fmt.Printf to output warnings directly to stdout is not ideal for library code. Consider one of these alternatives:

Use a logger interface that can be injected (e.g., log.Printf or a structured logger)

Collect warnings in the result structure and return them

Return warnings through the error return value using a custom error type that includes warnings

This would make the code more testable and allow callers to control how warnings are handled.

sehwan505 added 2 commits November 25, 2025 14:46

fix: update prompt for converter

ee289be

feat: enhance linter converters with validation and option registries

67949bf

sehwan505 requested a review from Copilot November 26, 2025 03:00

Copilot started reviewing on behalf of sehwan505 November 26, 2025 03:01 View session

Copilot finished reviewing on behalf of sehwan505 November 26, 2025 03:04

Copilot AI reviewed Nov 26, 2025

View reviewed changes

sehwan505 closed this Dec 1, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade converter with validation#30

feat: upgrade converter with validation#30
sehwan505 wants to merge 2 commits intomainfrom
feature/upgrade-linter

sehwan505 commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Copilot AI Nov 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		fmt.Printf("⚠️ Invalid ESLint rule '%s': %s\n", result.RuleName, validation.Message)
		return "", nil, nil

-	// TODO: Add options validation based on OptionsSchema
+	// Validate options against OptionsSchema
+	schema := def.Options
+	// If no options schema, accept any options
+	if schema.Type == "" {
+		return ValidationError{Valid: true}
+	}
+	// If schema expects an object, options should be a map[string]interface{}
+	if schema.Type == "object" {
+		optsMap, ok := options.(map[string]interface{})
+		if !ok {
+			return ValidationError{
+				Valid:      false,
+				Message:    "options should be an object (map)",
+				Suggestion: "Provide options as a key-value map.",
+			}
+		}
+		for propName, propSchema := range schema.Properties {
+			val, exists := optsMap[propName]
+			if !exists {
+				continue // Option is optional if not present
+			}
+			// Type check
+			switch propSchema.Type {
+			case "string":
+				_, ok := val.(string)
+				if !ok {
+					return ValidationError{
+						Valid:      false,
+						Message:    "option '" + propName + "' should be a string",
+						Suggestion: "Provide a string value for '" + propName + "'.",
+					}
+				}
+				// Enum check
+				if len(propSchema.Enum) > 0 {
+					strVal := val.(string)
+					valid := false
+					for _, allowed := range propSchema.Enum {
+						if strVal == allowed {
+							valid = true
+							break
+						}
+					}
+					if !valid {
+						return ValidationError{
+							Valid:      false,
+							Message:    "invalid value for '" + propName + "'",
+							Suggestion: "Valid values: " + joinStrings(propSchema.Enum),
+						}
+					}
+				}
+			case "array":
+				arr, ok := val.([]interface{})
+				if !ok {
+					return ValidationError{
+						Valid:      false,
+						Message:    "option '" + propName + "' should be an array",
+						Suggestion: "Provide an array value for '" + propName + "'.",
+					}
+				}
+				// Check item type
+				for _, item := range arr {
+					switch propSchema.Items {
+					case "string":
+						if _, ok := item.(string); !ok {
+							return ValidationError{
+								Valid:      false,
+								Message:    "array item in '" + propName + "' should be a string",
+								Suggestion: "All items in '" + propName + "' should be strings.",
+							}
+						}
+					// Add other types as needed
+					}
+				}
+			case "number":
+				_, ok := val.(float64)
+				if !ok {
+					return ValidationError{
+						Valid:      false,
+						Message:    "option '" + propName + "' should be a number",
+						Suggestion: "Provide a numeric value for '" + propName + "'.",
+					}
+				}
+			case "boolean":
+				_, ok := val.(bool)
+				if !ok {
+					return ValidationError{
+						Valid:      false,
+						Message:    "option '" + propName + "' should be a boolean",
+						Suggestion: "Provide a boolean value for '" + propName + "'.",
+					}
+				}
+			// Add other types as needed
+			}
+		}
+	}
+	// Add support for other schema types if needed

		fmt.Printf("⚠️ Invalid TSC option '%s': %s\n", key, validation.Message)
		}

+	// Validate type (all TSC options are boolean)
+	if _, ok := value.(bool); !ok {
+		return ValidationError{
+			Valid:   false,
+			Message: "expected boolean value for " + optionName,
+		}
+	}

		fmt.Printf("⚠️ Invalid Prettier option '%s': %s\n", key, validation.Message)
		}

Conversation

sehwan505 commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants