fix(linter): address PR #467 review feedback

- Add DropSequenceStatement case arm to L013 (drop_without_condition)
- Extend function_on_column visitor to traverse HAVING clauses and CTEs
- Remove unnecessary join := join copy in range loop
- Use neutral location instead of misleading sel.Pos in reserved_keyword_identifier
- Keep OR-to-IN threshold at 3 (consistent with existing tests)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: 2 people

pkg/linter/rules/naming/reserved_keyword_identifier.go

@@ -86,7 +86,7 @@ func (r *ReservedKeywordIdentifierRule) Check(ctx *linter.Context) ([]linter.Vio
 					RuleName:   r.Name(),
 					Severity:   r.Severity(),
 					Message:    "Table name '" + ref.Name + "' is a SQL reserved keyword",
-					Location:   models.Location{Line: sel.Pos.Line, Column: sel.Pos.Column},
+					Location:   models.Location{Line: 1, Column: 1},
 					Suggestion: "Quote the identifier: FROM \"" + ref.Name + "\" or rename the table",
 				})
 			}
@@ -96,7 +96,7 @@ func (r *ReservedKeywordIdentifierRule) Check(ctx *linter.Context) ([]linter.Vio
 					RuleName:   r.Name(),
 					Severity:   r.Severity(),
 					Message:    "Table alias '" + ref.Alias + "' is a SQL reserved keyword",
-					Location:   models.Location{Line: sel.Pos.Line, Column: sel.Pos.Column},
+					Location:   models.Location{Line: 1, Column: 1},
 					Suggestion: "Use a non-reserved alias instead of '" + ref.Alias + "'",
 				})
 			}

pkg/linter/rules/performance/function_on_column.go

@@ -86,30 +86,45 @@ func (v *functionOnColVisitor) Visit(node ast.Node) (ast.Visitor, error) {
 	if node == nil {
 		return nil, nil
 	}
-	// Track that we're inside a WHERE clause
+	// Track that we're inside a WHERE or HAVING clause
 	if sel, ok := node.(*ast.SelectStatement); ok {
+		child := &functionOnColVisitor{rule: v.rule, violations: v.violations, inWhere: false}
+		filterVisitor := &functionOnColVisitor{rule: v.rule, violations: v.violations, inWhere: true}
+
+		// Walk WHERE with inWhere=true
 		if sel.Where != nil {
-			child := &functionOnColVisitor{rule: v.rule, violations: v.violations, inWhere: false}
-			// Walk WHERE separately with inWhere=true
-			whereVisitor := &functionOnColVisitor{rule: v.rule, violations: v.violations, inWhere: true}
-			if err := ast.Walk(whereVisitor, sel.Where); err != nil {
+			if err := ast.Walk(filterVisitor, sel.Where); err != nil {
 				return nil, err
 			}
-			// Walk the rest normally (not in WHERE)
-			for _, col := range sel.Columns {
-				if err := ast.Walk(child, col); err != nil {
-					return nil, err
-				}
+		}
+		// Walk HAVING with inWhere=true (same index-breaking concern)
+		if sel.Having != nil {
+			if err := ast.Walk(filterVisitor, sel.Having); err != nil {
+				return nil, err
 			}
-			for _, join := range sel.Joins {
-				join := join
-				if err := ast.Walk(child, &join); err != nil {
-					return nil, err
+		}
+		// Walk CTEs to catch nested queries
+		if sel.With != nil {
+			for i := range sel.With.CTEs {
+				if sel.With.CTEs[i].Statement != nil {
+					if err := ast.Walk(v, sel.With.CTEs[i].Statement); err != nil {
+						return nil, err
+					}
 				}
 			}
-			return nil, nil // We've handled children manually
 		}
-		return v, nil
+		// Walk the rest normally (not in WHERE/HAVING)
+		for _, col := range sel.Columns {
+			if err := ast.Walk(child, col); err != nil {
+				return nil, err
+			}
+		}
+		for i := range sel.Joins {
+			if err := ast.Walk(child, &sel.Joins[i]); err != nil {
+				return nil, err
+			}
+		}
+		return nil, nil // We've handled children manually
 	}
 
 	if !v.inWhere {

pkg/linter/rules/safety/drop_without_condition.go

@@ -46,24 +46,38 @@ func (r *DropWithoutConditionRule) Check(ctx *linter.Context) ([]linter.Violatio
 	}
 	var violations []linter.Violation
 	for _, stmt := range ctx.AST.Statements {
-		drop, ok := stmt.(*ast.DropStatement)
-		if !ok {
-			continue
-		}
-		if !drop.IfExists {
-			objType := strings.ToUpper(drop.ObjectType)
-			name := ""
-			if len(drop.Names) > 0 {
-				name = drop.Names[0]
+		switch drop := stmt.(type) {
+		case *ast.DropStatement:
+			if !drop.IfExists {
+				objType := strings.ToUpper(drop.ObjectType)
+				name := ""
+				if len(drop.Names) > 0 {
+					name = drop.Names[0]
+				}
+				violations = append(violations, linter.Violation{
+					Rule:       r.ID(),
+					RuleName:   r.Name(),
+					Severity:   r.Severity(),
+					Message:    "DROP " + objType + " " + name + " is missing IF EXISTS",
+					Location:   models.Location{Line: 1, Column: 1},
+					Suggestion: "Use DROP " + objType + " IF EXISTS " + name,
+				})
+			}
+		case *ast.DropSequenceStatement:
+			if !drop.IfExists {
+				name := ""
+				if drop.Name != nil {
+					name = drop.Name.Name
+				}
+				violations = append(violations, linter.Violation{
+					Rule:       r.ID(),
+					RuleName:   r.Name(),
+					Severity:   r.Severity(),
+					Message:    "DROP SEQUENCE " + name + " is missing IF EXISTS",
+					Location:   drop.Pos,
+					Suggestion: "Use DROP SEQUENCE IF EXISTS " + name,
+				})
 			}
-			violations = append(violations, linter.Violation{
-				Rule:       r.ID(),
-				RuleName:   r.Name(),
-				Severity:   r.Severity(),
-				Message:    "DROP " + objType + " " + name + " is missing IF EXISTS",
-				Location:   models.Location{Line: 1, Column: 1},
-				Suggestion: "Use DROP " + objType + " IF EXISTS " + name,
-			})
 		}
 	}
 	return violations, nil