-
Notifications
You must be signed in to change notification settings - Fork 32
source: refactor code generator with TypeHandler class hierarchy #166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Introduce emit() helper to simplify emitting multi-line code blocks with proper indentation. This function uses textwrap.dedent() to allow writing code blocks naturally in Python while handling C indentation automatically. This is preparation for refactoring individual c_file.append() calls into consolidated multi-line f-strings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add free_and_null() helper function to consolidate the common pattern of freeing a pointer and setting it to NULL. This reduces code duplication and makes the intent more explicit. Replaced ~15 occurrences across the codebase where free() is followed immediately by setting the pointer to NULL. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add null_check_return() helper function to consolidate the common pattern of checking if a variable is NULL and returning NULL. This reduces code duplication and makes the intent more explicit. Replaced several occurrences in parse_map_string_obj() and parse_obj_type_array() functions. More occurrences remain to be replaced in subsequent commits. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace more occurrences of the NULL check pattern in parse_obj_type_array() for byte type handling. More replacements remain to be done in subsequent commits for full coverage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add calloc_with_check() helper function to consolidate the common pattern of calling calloc() followed by NULL checking and returning NULL on failure. This combines the calloc allocation with error handling in a single call, reducing code duplication and making the intent clearer. Replaced several occurrences in parse_map_string_obj() and parse_obj_type_array() functions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add check_gen_status() helper function to consolidate the common pattern of checking yajl_gen status and returning error on failure. This pattern appears ~50+ times in the codebase. Replaced the majority of occurrences across generate functions in get_map_string_obj(), get_obj_arr_obj_array(), and get_c_json(). A few occurrences with dynamic indentation (using level variable) remain and will be handled separately. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with consolidated multi-line f-strings using the emit() helper. This makes the C code generation more readable by showing the structure of the generated code directly. The helper functions (calloc_with_check, null_check_return) are retained for their specific patterns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with consolidated multi-line f-strings using the emit() helper. This function has three branches (object type, byte type, and other types), all now converted. The generated C code structure is now directly visible in the Python code, making it easier to understand and maintain. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with consolidated multi-line f-strings using the emit() helper. The check_gen_status() helper is strategically placed between code blocks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with consolidated multi-line f-strings in the object/subtypobj branch of get_obj_arr_obj_array(). The byte and else branches remain to be converted in subsequent commits. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Convert the remaining byte and else branches of get_obj_arr_obj_array() to use multi-line f-strings with the emit() helper. This completes the refactoring of this large, complex function that handles three different type branches. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with consolidated multi-line f-strings. Helper functions free_and_null() are retained for their specific patterns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with consolidated multi-line f-strings. Handles both single and double array cases cleanly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with consolidated multi-line f-strings. Handles multiple branches for basic maps, strings, compound types, and objects with both single and double array cases. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with consolidated multi-line f-strings. This function handles free() generation for different object types including mapStringObject, arrays, strings, and objects. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace individual c_file.append() calls with emit() helper using multi-line f-strings for better readability. This function handles parsing of different object types: string, number, boolean, object, array, and mapStringObject types. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Introduce emit_asprintf_error() helper function to emit the common pattern of asprintf with a strdup fallback for error handling. This pattern appears multiple times throughout the codebase. Apply the helper to the required field error check in parse_obj_arr_obj(). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Introduce emit_value_error() helper function to generate the common pattern of wrapping error messages with additional context about which key failed to parse. This pattern appears in multiple places when parsing map types. Apply the helper to parse_obj_type() and read_val_generator(). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Introduce emit_invalid_type_check() helper function to emit the common pattern of YAJL type validation with error return. This pattern appears when parsing numeric values. Apply the helper to read_val_generator() for numeric type parsing, reducing code duplication in the type validation logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Introduce helper functions for common YAJL JSON generation operations: - emit_gen_key(): Generate a JSON object key - emit_gen_map_open/close(): Generate map boundaries - emit_gen_array_open/close(): Generate array boundaries - emit_beautify_off/on(): Toggle JSON beautification These helpers will be used in subsequent commits to simplify the JSON generation code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Use the new YAJL generation helpers in get_map_string_obj() to simplify the code and improve readability: - emit_beautify_off/on() for beautify config - emit_gen_map_open/close() for map boundaries 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Use the YAJL generation helpers throughout get_obj_arr_obj_array() to simplify array generation code: - emit_gen_key() for generating key strings - emit_gen_array_open/close() for array boundaries - emit_beautify_off/on() for beautify config This reduces duplication and improves readability. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Use the YAJL generation helpers in get_c_json(): - emit_gen_map_open/close() for map boundaries - emit_beautify_off/on() for beautify config 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Use the YAJL generation helpers in get_c_epilog_for_array_make_gen(): - emit_gen_array_open/close() for array boundaries - Replace manual status checks with check_gen_status() where possible This simplifies the array epilog generation code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Use emit_gen_key() throughout get_obj_arr_obj() to simplify key generation code. This eliminates manual yajl_gen_string calls and length calculations for object keys. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add get_numeric_conversion_info() helper that returns the conversion function name and destination cast for numeric types. This eliminates the if-elif chain for selecting the appropriate common_safe_* function. Apply the helper to read_val_generator() to simplify numeric value parsing code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Remove the `if False:` block that was never executed. The code path in the else branch is now the only path. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Remove unnecessary `else:` keywords after `return` statements. When a branch ends with `return`, the `else:` is redundant. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Introduce a class-based approach for generating type-specific C code. Each type (string, boolean, numeric, etc.) has its own TypeHandler class with methods for: - emit_parse: parsing JSON to C struct - emit_generate: serializing C struct to JSON - emit_free: freeing memory - emit_clone: deep-copying - emit_read_value: reading a JSON value - emit_json_value: writing a JSON value The dispatch functions (read_val_generator, json_value_generator, get_obj_arr_obj) now use get_type_handler() to delegate to the appropriate handler. This makes the code more extensible and maintainable - adding a new type only requires adding a new handler class. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace the type-specific if/elif chain in parse_obj_type() with a call to the appropriate TypeHandler's emit_parse() method. Also remove the now-unused get_numeric_conversion_info() function since its logic is now part of the NumericType class. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Apply the TypeHandler pattern to make_c_free() and make_clone() for simple types (string, boolean, booleanPointer, numeric types). This delegates type-specific free and clone code generation to the appropriate handler classes, reducing the complexity of the dispatch logic in these functions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add TypeHandler classes for: - ObjectType: handles 'object' type parsing, generation, freeing - MapStringObjectType: handles 'mapStringObject' type - BasicMapType: handles basic map types (mapStringString, mapStringInt, etc.) Update get_type_handler() to return BasicMapType for valid basic maps. Simplify dispatch functions to use these handlers: - parse_obj_type: now only handles 'array' specially - get_obj_arr_obj: now only handles 'array' specially - json_value_generator: fully delegated to handlers Note: ObjectType.emit_clone() is a no-op because object cloning requires parent context (mapStringObject vs regular object). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Move array-specific code generation into ArrayType TypeHandler class: - emit_parse: delegates to parse_obj_type_array() - emit_generate: delegates to get_obj_arr_obj_array() - emit_free: delegates to make_c_array_free() - emit_clone: generates array clone code with support for numeric, object, and string subtypes including double arrays Remove explicit elif branches for array type in parse_obj_type(), get_obj_arr_obj(), make_c_free(), and make_clone() - these are now handled through the type handler dispatch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Move clone code generation for mapStringObject and basic map types (mapStringString, mapStringInt, etc.) into their TypeHandler classes: - MapStringObjectType.emit_clone: clones key-value pairs with values being complex objects - BasicMapType.emit_clone: delegates to clone_map_string_string or similar functions (note: clone functions don't use json_ prefix) Remove explicit elif branches for these types in make_clone() - now handled through the type handler dispatch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Remove explicit elif branches for basic map types, mapStringObject, and object types in make_c_free(). These are now fully handled by the type handler dispatch (BasicMapType, MapStringObjectType, ObjectType respectively). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Remove explicit elif branch for basic map types since this is now handled by BasicMapType.emit_read_value() via the type handler dispatch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Object type clone requires parent context to determine whether to emit simple clone code (for regular object parents) or array iteration clone code (for mapStringObject parents). Skip the handler dispatch for object type and use explicit handling instead. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Rename functions with confusing "judge_" prefix to more descriptive names following Python naming conventions: - judge_data_type -> is_numeric_type - judge_data_pointer_type -> is_numeric_pointer_type - obtain_data_pointer_type -> get_pointer_base_type Also simplify the docstrings to be more concise and informative. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Move parse_obj_type_array, get_obj_arr_obj_array, and make_c_array_free directly into the ArrayType class methods they were called from. This consolidates all array-related code generation into the ArrayType handler, making the code more cohesive and easier to maintain. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Remove parse_obj_type and get_obj_arr_obj wrapper functions by inlining their single-line bodies directly at the call sites. These wrappers just called get_type_handler() and delegated to the handler methods, adding unnecessary indirection. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Move the c_file_str() function body directly into the ArrayType.emit_free() method since it was only called from there. This consolidates all array string freeing logic within the ArrayType handler. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Move the c_file_map_str() function body directly into make_c_free() since it was only called from there. This removes unnecessary indirection for mapStringObject free logic. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Replace duplicate type-case dispatch logic in make_clone() and make_c_free() with a new get_compound_children() helper function. Both functions had identical dictionary-based dispatch to get children/subtypes for compound types. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add a combined helper that calls emit_gen_key() followed by check_gen_status(), replacing 10 occurrences of this common pattern throughout the codebase. This reduces boilerplate and makes the code more concise. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Improve the comment explaining why ObjectType.emit_clone() is intentionally empty. The actual object cloning is handled directly in make_clone() because it requires per-type context that the handler dispatch pattern doesn't provide. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Complete the BooleanPointerType handler by adding emit_generate() and emit_json_value() methods. These were missing, which meant booleanPointer fields couldn't be serialized to JSON. The implementation follows the same pattern as NumericPointerType. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Split the monolithic ArrayType class into specialized subtype handlers: - ObjectArrayHandler: handles arrays of objects - ByteArrayHandler: handles byte arrays - PrimitiveArrayHandler: handles arrays of strings, numerics, etc. - BasicMapArrayHandler: handles arrays of basic map types The ArrayType class now delegates to _get_array_subtype_handler() which returns the appropriate handler based on the array's element type. This follows the same pattern used for the TypeHandler hierarchy and makes each subtype's code generation logic self-contained. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add emit_make_body(), emit_gen_body(), emit_free_body(), and emit_clone_body() methods to ObjectType, MapStringObjectType, and ArrayType classes. These methods encapsulate the struct-level code generation logic that was previously scattered across orchestrator functions. Refactor parse_json_to_c(), get_c_json(), make_clone(), and make_c_free() to delegate to these new methods, simplifying the orchestrator functions to just handle function signature generation and the common prologue/epilogue code. Remove the now-unused parse_map_string_obj(), parse_obj_arr_obj(), and get_map_string_obj() helper functions since their logic is now encapsulated in the TypeHandler classes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Rename the 'doublearray' attribute to 'nested_array' for clarity. The previous name was confusing as it could be mistaken for the 'double' floating-point type, when it actually indicates an array of arrays (2D/nested array structure). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
bbaeb8a to
a425a84
Compare
flouthoc
approved these changes
Dec 13, 2025
Collaborator
flouthoc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
this was long overdue but too painful to do manually and not important enough since the end result is almost the same.
Most of the code was written By Claude Code.
🤖 Generated with Claude Code
@flouthoc PTAL