From a4064ccba8f0080063609589f69d7162af6b785c Mon Sep 17 00:00:00 2001 From: tsanders Date: Fri, 7 Nov 2025 11:46:07 -0500 Subject: [PATCH 1/3] fix: Gracefully skip rules requiring unavailable providers When analyzing a codebase with a subset of providers (e.g., analyzing TypeScript code with only nodejs and builtin providers), rules that require unavailable providers (e.g., Java provider) would generate hundreds of error log messages like: "unable to find provider for: java" This polluted logs and made it difficult to identify real issues. This commit introduces a MissingProviderError type that allows the rule parser to distinguish between "provider not available" (which is expected and should be handled gracefully) and actual parsing errors (which should be logged). Changes: - Add MissingProviderError custom error type - Update getConditionForProvider to return MissingProviderError - Update getConditions and LoadRule to catch MissingProviderError and skip conditions/rules gracefully with debug-level logging - Update "rule no provider" test to expect success (rule skipped) instead of error Before: 217 error messages when analyzing tackle2-ui with nodejs After: 0 error messages - rules are silently skipped at debug level Fixes: konveyor/analyzer-lsp#956 Signed-off-by: tsanders --- parser/rule_parser.go | 33 ++++++++++++++++++++++++++++++--- parser/rule_parser_test.go | 6 ++++-- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/parser/rule_parser.go b/parser/rule_parser.go index 4e08decb..8574d21e 100644 --- a/parser/rule_parser.go +++ b/parser/rule_parser.go @@ -23,6 +23,15 @@ var defaultRuleSet = &engine.RuleSet{ Name: "konveyor-analysis", } +// MissingProviderError indicates a rule requires a provider that is not available +type MissingProviderError struct { + Provider string +} + +func (e MissingProviderError) Error() string { + return fmt.Sprintf("unable to find provider for: %v", e.Provider) +} + type parserErrors struct { errs []error } @@ -348,7 +357,10 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid } conditions, provs, err := r.getConditions(m) if err != nil { - r.Log.V(8).Error(err, "failed parsing conditions in or clause", "ruleID", ruleID, "file", filepath) + // Skip logging for missing provider errors - they're already logged at debug level + if _, ok := err.(MissingProviderError); !ok { + r.Log.V(8).Error(err, "failed parsing conditions in or clause", "ruleID", ruleID, "file", filepath) + } return nil, nil, err } if len(conditions) == 0 { @@ -377,7 +389,10 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid } conditions, provs, err := r.getConditions(m) if err != nil { - r.Log.V(8).Error(err, "failed parsing conditions in and clause", "ruleID", ruleID, "file", filepath) + // Skip logging for missing provider errors - they're already logged at debug level + if _, ok := err.(MissingProviderError); !ok { + r.Log.V(8).Error(err, "failed parsing conditions in and clause", "ruleID", ruleID, "file", filepath) + } return nil, nil, err } if len(conditions) == 0 { @@ -410,6 +425,12 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid condition, provider, err := r.getConditionForProvider(providerKey, capability, value) if err != nil { + // If provider is missing, log at debug level and skip this rule + if _, ok := err.(MissingProviderError); ok { + r.Log.V(5).Info("skipping rule for unavailable provider", "provider", providerKey, "capability", capability, "ruleID", ruleID) + continue + } + // For other errors, log and return as before r.Log.V(8).Error(err, "failed parsing conditions for provider", "provider", providerKey, "capability", capability, "ruleID", ruleID, "file", filepath) return nil, nil, err @@ -694,6 +715,12 @@ func (r *RuleParser) getConditions(conditionsInterface []interface{}) ([]engine. condition, provider, err := r.getConditionForProvider(providerKey, capability, v) if err != nil { + // If provider is missing, log at debug level and skip this condition + if _, ok := err.(MissingProviderError); ok { + r.Log.V(5).Info("skipping condition for unavailable provider", "provider", providerKey, "capability", capability) + continue + } + // For other errors, return as before return nil, nil, err } if condition == nil { @@ -744,7 +771,7 @@ func (r *RuleParser) getConditionForProvider(langProvider, capability string, va // Here there can only be a single provider. client, ok := r.ProviderNameToClient[langProvider] if !ok { - return nil, nil, fmt.Errorf("unable to find provider for: %v", langProvider) + return nil, nil, MissingProviderError{Provider: langProvider} } if !provider.HasCapability(client.Capabilities(), capability) { diff --git a/parser/rule_parser_test.go b/parser/rule_parser_test.go index e215bb3d..e52f6b49 100644 --- a/parser/rule_parser_test.go +++ b/parser/rule_parser_test.go @@ -361,8 +361,10 @@ func TestLoadRules(t *testing.T) { }}, }, }, - ShouldErr: true, - ErrorMessage: "unable to find provider for: builtin", + // With the fix, missing providers are gracefully skipped, not errors + // The rule will be skipped since provider is unavailable + ShouldErr: false, + ErrorMessage: "", }, { Name: "rule no conditions", From cd795756226fd011ec3cf9857e166bbc5a9c3240 Mon Sep 17 00:00:00 2001 From: tsanders Date: Fri, 7 Nov 2025 14:32:50 -0500 Subject: [PATCH 2/3] fix: Don't return MissingProviderError in or/and clauses Previously, MissingProviderError was still being returned in or/and clauses even though error logging was suppressed. This caused the errors to bubble up to LoadRules and get aggregated in parserErr, resulting in error-level logs. Now, when MissingProviderError is encountered in or/and clauses, we set noConditions=true and break, which causes the rule to be gracefully skipped without any error propagation. This eliminates all error-level logging for missing providers. Signed-off-by: tsanders --- parser/rule_parser.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/parser/rule_parser.go b/parser/rule_parser.go index 8574d21e..059d9040 100644 --- a/parser/rule_parser.go +++ b/parser/rule_parser.go @@ -357,10 +357,13 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid } conditions, provs, err := r.getConditions(m) if err != nil { - // Skip logging for missing provider errors - they're already logged at debug level - if _, ok := err.(MissingProviderError); !ok { - r.Log.V(8).Error(err, "failed parsing conditions in or clause", "ruleID", ruleID, "file", filepath) + // If provider is missing, treat as no conditions and skip the rule + if _, ok := err.(MissingProviderError); ok { + noConditions = true + break } + // For other errors, log and return as before + r.Log.V(8).Error(err, "failed parsing conditions in or clause", "ruleID", ruleID, "file", filepath) return nil, nil, err } if len(conditions) == 0 { @@ -389,10 +392,13 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid } conditions, provs, err := r.getConditions(m) if err != nil { - // Skip logging for missing provider errors - they're already logged at debug level - if _, ok := err.(MissingProviderError); !ok { - r.Log.V(8).Error(err, "failed parsing conditions in and clause", "ruleID", ruleID, "file", filepath) + // If provider is missing, treat as no conditions and skip the rule + if _, ok := err.(MissingProviderError); ok { + noConditions = true + break } + // For other errors, log and return as before + r.Log.V(8).Error(err, "failed parsing conditions in and clause", "ruleID", ruleID, "file", filepath) return nil, nil, err } if len(conditions) == 0 { From 9a52a2d6cae2027197a73dca1f30ab9c939d19db Mon Sep 17 00:00:00 2001 From: tsanders Date: Fri, 7 Nov 2025 14:56:32 -0500 Subject: [PATCH 3/3] fix: Prevent incorrect evaluation of AND/OR clauses with partial conditions Remove unreachable MissingProviderError handlers and fix condition filtering logic to handle mixed-provider rules correctly: - OR clauses now only skip when ALL conditions are filtered, allowing partial OR clauses to proceed with surviving branches (e.g., "java.referenced OR nodejs.referenced" evaluates as "nodejs.referenced" when Java provider is unavailable) - AND clauses skip when ANY condition is filtered, since all conditions are required for AND logic - Nested AND/OR conditions use 'continue' to skip invalid branches while allowing other valid branches to be collected This prevents both incorrect rule evaluation from partial conditions and unnecessarily dropping valid OR branches. Signed-off-by: tsanders --- parser/rule_parser.go | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/parser/rule_parser.go b/parser/rule_parser.go index 059d9040..99037966 100644 --- a/parser/rule_parser.go +++ b/parser/rule_parser.go @@ -357,16 +357,11 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid } conditions, provs, err := r.getConditions(m) if err != nil { - // If provider is missing, treat as no conditions and skip the rule - if _, ok := err.(MissingProviderError); ok { - noConditions = true - break - } - // For other errors, log and return as before r.Log.V(8).Error(err, "failed parsing conditions in or clause", "ruleID", ruleID, "file", filepath) return nil, nil, err } if len(conditions) == 0 { + r.Log.V(5).Info("skipping rule due to missing providers in or clause", "ruleID", ruleID, "expected", len(m), "actual", len(conditions)) noConditions = true } @@ -392,16 +387,14 @@ func (r *RuleParser) LoadRule(filepath string) ([]engine.Rule, map[string]provid } conditions, provs, err := r.getConditions(m) if err != nil { - // If provider is missing, treat as no conditions and skip the rule - if _, ok := err.(MissingProviderError); ok { - noConditions = true - break - } - // For other errors, log and return as before r.Log.V(8).Error(err, "failed parsing conditions in and clause", "ruleID", ruleID, "file", filepath) return nil, nil, err } - if len(conditions) == 0 { + // Skip rule if some or all conditions were filtered due to missing providers + if len(conditions) != len(m) { + if len(conditions) > 0 { + r.Log.V(5).Info("skipping rule due to partial condition filtering in and clause", "ruleID", ruleID, "expected", len(m), "actual", len(conditions)) + } noConditions = true } rule.When = engine.AndCondition{Conditions: conditions} @@ -665,10 +658,8 @@ func (r *RuleParser) getConditions(conditionsInterface []interface{}) ([]engine. if err != nil { return nil, nil, err } - // There was no error so the conditions have all been filtered - // Return early to prevent constructing an empty rule - if len(conds) == 0 && len(conds) != len(iConditions) { - return []engine.ConditionEntry{}, nil, nil + if len(conds) != len(iConditions) { + continue } ce = engine.ConditionEntry{ From: from, @@ -691,10 +682,8 @@ func (r *RuleParser) getConditions(conditionsInterface []interface{}) ([]engine. if err != nil { return nil, nil, err } - // There was no error so the conditions have all been filtered - // Return early to prevent constructing an empty rule - if len(conds) == 0 && len(conds) != len(iConditions) { - return []engine.ConditionEntry{}, nil, nil + if len(conds) == 0 { + continue } ce = engine.ConditionEntry{ From: from,