New Analyzer Rule: Detect Usage of Generated Client Models in Controller Responses (AG0050)

[joeldickson]

New Analyzer Rule: Detect Usage of Generated Client Models in Controller Responses

🎯 Objective

Create a new Roslyn analyzer rule to prevent tight coupling between data/dependency layer models and API response models by detecting when generated client models are used directly in controller action responses.

📋 Background & Motivation

Currently, there's a risk of tight coupling when developers use generated client models (from HTTP clients or GraphQL clients) directly in controller responses. This creates several issues:

• Tight Coupling: API responses become directly dependent on external service contracts
• Breaking Changes: Changes in external services can break our API responses unexpectedly
• Poor Separation of Concerns: Blurs the boundary between data access and API presentation layers
• Maintenance Issues: Makes it harder to evolve APIs independently

🔍 Detection Strategy

The rule should identify when models from generated client assemblies are used anywhere in controller actions by:

1. Assembly Name Detection Possible due to naming convention at Agoda:

   • HTTP generated clients: Assembly names ending with .Client
   • GraphQL generated clients: Assembly names ending with .Graphql

2. Complete Object Tree Walking:

   • Response Analysis: Analyze the complete object graph of controller action return types
   • Parameter Analysis: Analyze all input parameters to controller actions
   • Deep Property Traversal: Recursively examine all properties, nested objects, collections, and generic type parameters
   • Multi-Level Detection: Detect generated client models at any depth within the object structure

📝 Detailed Requirements

Scope

• Target: Controller action methods (classes inheriting from Controller or ControllerBase)
• Analysis Depth: Full object tree traversal of both return types AND input parameters
• Property Traversal: Deep analysis of all properties within response/parameter classes
• Coverage: Both synchronous and asynchronous action methods

Detection Logic

// ✅ GOOD - Using dedicated response models with clean properties
public ActionResult<UserResponse> GetUser(int id)
{
    return Ok(new UserResponse { Name = "John", Age = 30 });
}

public class UserResponse 
{
    public string Name { get; set; }
    public int Age { get; set; }
}

// ❌ BAD - Using generated client model directly in return type
public ActionResult<UserService.Client.UserModel> GetUser(int id)
{
    return Ok(generatedClientModel);
}

// ❌ BAD - Generated client model as property in response class
public ActionResult<UserResponse> GetUser(int id)
{
    return Ok(new UserResponse());
}

public class UserResponse 
{
    public UserService.Client.UserDto User { get; set; } // ← VIOLATION HERE
    public string AdditionalInfo { get; set; }
}

// ❌ BAD - Generated client model nested deeper in response structure
public ActionResult<ApiWrapper> GetUser(int id)
{
    return Ok(new ApiWrapper());
}

public class ApiWrapper
{
    public UserData Data { get; set; }
}

public class UserData 
{
    public UserService.Client.UserDto Details { get; set; } // ← VIOLATION HERE
    public List<Orders.Graphql.OrderModel> Orders { get; set; } // ← VIOLATION HERE
}

// ❌ BAD - Generated client model in input parameters
public ActionResult<UserResponse> UpdateUser(UserService.Client.UserDto userInput) // ← VIOLATION HERE
{
    return Ok(new UserResponse());
}

// ❌ BAD - Generated client model in complex input parameter
public ActionResult<UserResponse> UpdateUser(UpdateRequest request) // ← Contains violation
{
    return Ok(new UserResponse());
}

public class UpdateRequest
{
    public UserService.Client.UserDto UserData { get; set; } // ← VIOLATION HERE
    public string Reason { get; set; }
}

// ❌ BAD - Generated model in collection within response property
public ActionResult<OrderSummary> GetOrderSummary()
{
    return Ok(new OrderSummary());
}

public class OrderSummary
{
    public List<Orders.Graphql.OrderModel> RecentOrders { get; set; } // ← VIOLATION HERE
    public decimal TotalAmount { get; set; }
}

Assembly/Namespace Detection

• Primary: Use Type.Assembly.FullName or Type.Assembly.GetName().Name
• Fallback: Use Type.Namespace if assembly name detection fails
• Patterns to Match:
  • *.Client (case-insensitive)
  • *.Graphql (case-insensitive)

🛠 Implementation Approach

Analyzer Structure

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class GeneratedClientModelInResponseAnalyzer : DiagnosticAnalyzer
{
    public static readonly DiagnosticDescriptor Rule = new DiagnosticDescriptor(
        "AG001", // Rule ID
        "Generated client model used in controller response",
        "Controller action '{0}' returns generated client model '{1}' from assembly '{2}'",
        "Design",
        DiagnosticSeverity.Warning, // or Error based on preference
        isEnabledByDefault: true
    );
}

Key Components

1. Controller Detection: Identify controller classes and action methods
2. Return Type Analysis: Extract and analyze return types from action methods
3. Parameter Type Analysis: Extract and analyze all input parameter types from action methods
4. Deep Type Tree Walker: Recursively examine all types, properties, and nested structures
5. Property Traversal: Walk through all properties of classes to detect violations at any depth
6. Assembly Pattern Matcher: Check if types come from generated client assemblies
7. Diagnostic Reporter: Report violations with detailed context including the exact property path

Type Tree Walking Algorithm

private bool ContainsGeneratedClientModel(ITypeSymbol type, HashSet<ITypeSymbol> visited)
{
    if (visited.Contains(type)) return false;
    visited.Add(type);
    
    // Check current type
    if (IsGeneratedClientModel(type)) return true;
    
    // Check generic type arguments
    if (type is INamedTypeSymbol namedType)
    {
        foreach (var typeArg in namedType.TypeArguments)
        {
            if (ContainsGeneratedClientModel(typeArg, visited)) return true;
        }
    }
    
    // Check properties
    foreach (var member in type.GetMembers().OfType<IPropertySymbol>())
    {
        if (ContainsGeneratedClientModel(member.Type, visited)) return true;
    }
    
    return false;
}

✅ Acceptance Criteria

Must Detect

• Direct usage of generated client models as return types
• Generated client models nested in generic types (ActionResult<T>, Task<T>, etc.)
• Generated client models in collection types (List<T>, IEnumerable<T>, etc.)
• Generated client models in custom response wrapper classes
• Generated client models as properties within response classes (any depth)
• Generated client models in input parameters (direct and nested)
• Generated client models as properties within parameter classes (any depth)
• Generated client models in deeply nested object structures
• Both HTTP client models (.Client assemblies) and GraphQL models (.Graphql assemblies)
• Generated client models in collections within properties (List<GeneratedModel> as a property)

Must Handle

• Async action methods (Task<ActionResult<T>>)
• Various ActionResult types (Ok(), BadRequest(), NotFound(), etc.)
• Complex generic nested structures
• Circular references in object graphs (prevent infinite loops)
• Nullable reference types
• Abstract base controllers

Should Not Flag

• Generated client models used in private methods that are not controller actions
• Generated client models used in non-controller classes
• Legitimate response/parameter models that happen to have similar naming patterns but aren't from generated assemblies
• Generated client models used internally within business logic (not in controller signatures)

🧪 Test Cases

Positive Cases (Should Trigger Rule)

// Direct usage in return type
public ActionResult<UserService.Client.UserDto> GetUser() { }

// Nested in ActionResult
public async Task<ActionResult<Orders.Graphql.OrderModel>> GetOrder() { }

// In collections
public ActionResult<List<Products.Client.ProductModel>> GetProducts() { }

// CRITICAL: Generated client model as property in response class
public ActionResult<UserResponse> GetUser() { }
public class UserResponse
{
    public UserService.Client.UserDto User { get; set; } // ← MUST DETECT
}

// CRITICAL: Multiple levels deep in response properties
public ActionResult<ApiWrapper> GetData() { }
public class ApiWrapper
{
    public DataContainer Data { get; set; }
}
public class DataContainer 
{
    public UserService.Client.UserDto UserInfo { get; set; } // ← MUST DETECT
    public List<Orders.Graphql.OrderModel> Orders { get; set; } // ← MUST DETECT
}

// CRITICAL: Generated client models in input parameters
public ActionResult<UserResponse> UpdateUser(UserService.Client.UserDto input) { } // ← MUST DETECT

// CRITICAL: Generated client models in parameter properties
public ActionResult<UserResponse> UpdateUser(UpdateRequest request) { } // ← MUST DETECT
public class UpdateRequest
{
    public UserService.Client.UserDto UserData { get; set; } // ← MUST DETECT
    public string Reason { get; set; }
}

// Deep nesting in both parameters and responses
public ActionResult<PagedResult<List<UserService.Client.UserDto>>> GetPagedUsers(
    SearchRequest<Products.Client.ProductFilter> searchParams) { } // ← Both should be detected

Negative Cases (Should Not Trigger Rule)

// Dedicated response models with clean properties
public ActionResult<UserResponse> GetUser() { }
public class UserResponse 
{
    public string Name { get; set; }
    public int Age { get; set; }
    // No generated client models in properties
}

// Dedicated input models with clean properties
public ActionResult<UserResponse> UpdateUser(UpdateUserRequest request) { }
public class UpdateUserRequest 
{
    public string Name { get; set; }
    public int Age { get; set; }
    // No generated client models in properties
}

// Non-controller classes (business logic)
public class UserService 
{
    public UserService.Client.UserDto GetUser() { } // OK - not a controller
    private void ProcessUser(UserService.Client.UserDto user) { } // OK - not a controller action
}

// Models with similar naming but from legitimate assemblies
public ActionResult<UserResponse> GetUser() { }
public class UserResponse 
{
    public MyApp.Client.UserDto User { get; set; } // OK - not generated (different assembly pattern)
}

🏷 Metadata

• Rule ID: AG0050 (or next available ID in sequence)
• Category: Design / Architecture
• Severity: Warning (configurable to Error if needed)
• Title: "Generated client model used in controller action"
• Description: "Controller actions should not use generated client models in parameters or return types (including nested properties) to avoid tight coupling"

Note: Dont forget to write the unit tests first

[joeldickson]

This is an Agoda Specific rule, due to the naming convention that is only consistent with us, we should break into a separate project