From 8c1f88fb82c75ea03c43cc8cf8a3b0939734c6c4 Mon Sep 17 00:00:00 2001 From: Mark VanderVoord Date: Thu, 18 Jun 2026 12:50:29 -0400 Subject: [PATCH] Improve the automatic detection of argument pairs related to arrays/pointers and lengths --- lib/cmock_generator_plugin_array.rb | 10 +- lib/cmock_generator_utils.rb | 30 ++++- lib/cmock_header_parser.rb | 55 +++++++- .../test_interactions/array_handling.yml | 57 ++++++++ test/unit/cmock_generator_utils_test.rb | 4 +- test/unit/cmock_header_parser_test.rb | 123 ++++++++++++++++++ 6 files changed, 260 insertions(+), 19 deletions(-) diff --git a/lib/cmock_generator_plugin_array.rb b/lib/cmock_generator_plugin_array.rb index da05bf6..ae250a3 100644 --- a/lib/cmock_generator_plugin_array.rb +++ b/lib/cmock_generator_plugin_array.rb @@ -28,10 +28,10 @@ class CMockGeneratorPluginArray def mock_function_declarations(function) return nil unless function[:contains_ptr?] - args_call_i = function[:args].map { |m| m[:ptr?] ? "#{m[:name]}, #{m[:name]}_Depth" : (m[:name]).to_s }.join(', ') - args_call_o = function[:args].map { |m| m[:ptr?] ? "#{m[:name]}, (#{m[:name]}_Depth)" : (m[:name]).to_s }.join(', ') + args_call_i = function[:args].map { |m| m[:ptr?] && m[:array_size_order] != :before ? "#{m[:name]}, #{m[:name]}_Depth" : (m[:name]).to_s }.join(', ') + args_call_o = function[:args].map { |m| m[:ptr?] && m[:array_size_order] != :before ? "#{m[:name]}, (#{m[:name]}_Depth)" : (m[:name]).to_s }.join(', ') args_string = function[:args].map do |m| - m[:ptr?] ? "#{@utils.arg_declaration(m)}, int #{m[:name]}_Depth" : @utils.arg_declaration(m) + m[:ptr?] && m[:array_size_order] != :before ? "#{@utils.arg_declaration(m)}, int #{m[:name]}_Depth" : @utils.arg_declaration(m) end.join(', ') lines = '' if function[:return][:void?] @@ -52,9 +52,9 @@ class CMockGeneratorPluginArray lines = [] func_name = function[:name] args_string = function[:args].map do |m| - m[:ptr?] ? "#{@utils.arg_declaration(m)}, int #{m[:name]}_Depth" : @utils.arg_declaration(m) + m[:ptr?] && m[:array_size_order] != :before ? "#{@utils.arg_declaration(m)}, int #{m[:name]}_Depth" : @utils.arg_declaration(m) end.join(', ') - call_string = function[:args].map { |m| m[:ptr?] ? "#{m[:name]}, #{m[:name]}_Depth" : m[:name] }.join(', ') + call_string = function[:args].map { |m| m[:ptr?] && m[:array_size_order] != :before ? "#{m[:name]}, #{m[:name]}_Depth" : m[:name] }.join(', ') lines << if function[:return][:void?] "void #{func_name}_CMockExpectWithArray(UNITY_LINE_TYPE cmock_line, #{args_string})\n" else diff --git a/lib/cmock_generator_utils.rb b/lib/cmock_generator_utils.rb index 029b83e..471e547 100644 --- a/lib/cmock_generator_utils.rb +++ b/lib/cmock_generator_utils.rb @@ -83,7 +83,7 @@ class CMockGeneratorUtils def code_add_an_arg_expectation(arg, depth = 1) lines = code_assign_argument_quickly("cmock_call_instance->Expected_#{arg[:name]}", arg) - lines << " cmock_call_instance->Expected_#{arg[:name]}_Depth = #{arg[:name]}_Depth;\n" if @arrays && depth.instance_of?(String) + lines << " cmock_call_instance->Expected_#{arg[:name]}_Depth = #{depth};\n" if @arrays && depth.instance_of?(String) lines << " cmock_call_instance->IgnoreArg_#{arg[:name]} = 0;\n" if @ignore_arg lines << " cmock_call_instance->ReturnThruPtr_#{arg[:name]}_Used = 0;\n" if @return_thru_ptr && ptr_or_str?(arg[:type]) && !(arg[:const?]) lines @@ -105,11 +105,27 @@ class CMockGeneratorUtils if function[:args_string] != 'void' if @arrays args_string = function[:args].map do |m| - m[:ptr?] ? "#{arg_declaration(m)}, int #{m[:name]}_Depth" : arg_declaration(m) + if m[:ptr?] && m[:array_size_order] == :before + arg_declaration(m) + elsif m[:ptr?] + "#{arg_declaration(m)}, int #{m[:name]}_Depth" + else + arg_declaration(m) + end end.join(', ') + body = function[:args].inject('') do |all, arg| + depth = if arg[:ptr?] && arg[:array_size_order] == :before + arg[:array_size_name] + elsif arg[:ptr?] + "#{arg[:name]}_Depth" + else + 1 + end + all + code_add_an_arg_expectation(arg, depth) + end "void CMockExpectParameters_#{function[:name]}(CMOCK_#{function[:name]}_CALL_INSTANCE* cmock_call_instance, #{args_string});\n" \ "void CMockExpectParameters_#{function[:name]}(CMOCK_#{function[:name]}_CALL_INSTANCE* cmock_call_instance, #{args_string})\n{\n" \ - "#{function[:args].inject('') { |all, arg| all + code_add_an_arg_expectation(arg, (arg[:ptr?] ? "#{arg[:name]}_Depth" : 1)) }}" \ + "#{body}" \ "}\n\n" else "void CMockExpectParameters_#{function[:name]}(CMOCK_#{function[:name]}_CALL_INSTANCE* cmock_call_instance, #{function[:args_string]});\n" \ @@ -125,10 +141,14 @@ class CMockGeneratorUtils def code_call_argument_loader(function) if function[:args_string] != 'void' args = function[:args].map do |m| - if @arrays && m[:ptr?] && !(m[:array_data?]) + if @arrays && m[:ptr?] && m[:array_size_order] == :after + "#{m[:name]}, #{m[:array_size_name]}" + elsif @arrays && m[:ptr?] && m[:array_size_order] == :before + m[:name] + elsif @arrays && m[:ptr?] "#{m[:name]}, 1" elsif @arrays && m[:array_size?] - "#{m[:name]}, #{m[:name]}" + m[:name] else m[:name] end diff --git a/lib/cmock_header_parser.rb b/lib/cmock_header_parser.rb index ae418c9..6769513 100644 --- a/lib/cmock_header_parser.rb +++ b/lib/cmock_header_parser.rb @@ -456,20 +456,61 @@ class CMockHeaderParser args << arg_info end - # Try to find array pair in parameters following this pattern : * , <@array_size_type> <@array_size_name> - args.each_with_index do |val, index| - next_index = index + 1 - next unless args.length > next_index + # Try to find array pairs using name-affinity scoring. + # Pairs each size-candidate arg with the best-matching pointer arg. + # Score: 10=exact root match, 7=prefix match, 5=substring match, +2 adjacency bonus (ptr immediately precedes size). + # Falls back to adjacency alone (score 2) when no name affinity found, preserving original behavior. + size_candidate_indices = args.each_index.select do |i| + args[i][:name].match(@array_size_name) && @array_size_type.include?(args[i][:type]) + end - if (val[:ptr?] == true) && args[next_index][:name].match(@array_size_name) && @array_size_type.include?(args[next_index][:type]) - val[:array_data?] = true - args[next_index][:array_size?] = true + size_candidate_indices.each do |size_idx| + best_score = 0 + best_ptr_idx = nil + + args.each_with_index do |ptr_arg, ptr_idx| + next unless ptr_arg[:ptr?] && !ptr_arg[:array_data?] + + score = array_size_name_affinity(ptr_arg[:name], args[size_idx][:name]) + score += 2 if ptr_idx + 1 == size_idx + + if score > best_score + best_score = score + best_ptr_idx = ptr_idx + end end + + next unless best_ptr_idx + + args[best_ptr_idx][:array_size_name] = args[size_idx][:name] + args[best_ptr_idx][:array_size_order] = size_idx < best_ptr_idx ? :before : :after + args[size_idx][:array_size?] = true end args end + def array_size_name_affinity(ptr_name, size_name) + size_words = @array_size_name.to_s.split('|').select { |w| w.match?(/^\w+$/) } + p_name = ptr_name.downcase + s_name = size_name.downcase + + roots = size_words.flat_map do |word| + [ + s_name.sub(/_#{Regexp.escape(word)}$/, ''), + s_name.sub(/^#{Regexp.escape(word)}_/, '') + ] + end.uniq.reject { |r| r.empty? || r == s_name } + + roots.each do |root| + return 10 if root == p_name + return 7 if p_name.start_with?(root) || root.start_with?(p_name) + return 5 if p_name.include?(root) || root.include?(p_name) + end + + 0 + end + def divine_ptr(arg) return false unless arg.include? '*' # treat "const char *" and similar as a string, not a pointer diff --git a/test/system/test_interactions/array_handling.yml b/test/system/test_interactions/array_handling.yml index 49f5f51..03d4895 100644 --- a/test/system/test_interactions/array_handling.yml +++ b/test/system/test_interactions/array_handling.yml @@ -50,6 +50,7 @@ void write_buffer(char (*dest)[10]); void read_buffers(int count, char (*buffers)[8]); void process_handles(void *const handles[], int count); + void store_data(int *written_count, int buf_size, int *buf); :source: :header: | @@ -63,6 +64,7 @@ void function_h(void); void function_i(void); void function_j(void); + void function_k(void); :code: | void function_a(void) @@ -136,6 +138,13 @@ process_handles(ptrs, 3); } + void function_k(void) + { + int count = 0; + int data[] = {10, 20, 30}; + store_data(&count, 3, data); + } + :tests: :common: | void setUp(void) {} @@ -639,4 +648,52 @@ function_j(); } + - :pass: TRUE + :should: 'pair buf_size with buf not written_count, and pass' + :code: | + test() + { + int exp_count = 0; + int exp_data[] = {10, 20, 30}; + store_data_Expect(&exp_count, 3, exp_data); + + function_k(); + } + + - :pass: FALSE + :should: 'detect mismatch in buf array when buf_size is correctly paired with buf' + :code: | + test() + { + int exp_count = 0; + int exp_data[] = {10, 20, 99}; + store_data_Expect(&exp_count, 3, exp_data); + + function_k(); + } + + - :pass: TRUE + :should: 'use ExpectWithArray with length before pointer order' + :code: | + test() + { + int exp_count = 0; + int exp_data[] = {10, 20, 30}; + store_data_ExpectWithArray(&exp_count, 1, 3, exp_data); + + function_k(); + } + + - :pass: FALSE + :should: 'detect content mismatch in buf via ExpectWithArray' + :code: | + test() + { + int exp_count = 0; + int exp_data[] = {10, 99, 30}; + store_data_ExpectWithArray(&exp_count, 1, 3, exp_data); + + function_k(); + } + ... diff --git a/test/unit/cmock_generator_utils_test.rb b/test/unit/cmock_generator_utils_test.rb index b05f4d3..b16d4b2 100644 --- a/test/unit/cmock_generator_utils_test.rb +++ b/test/unit/cmock_generator_utils_test.rb @@ -130,7 +130,7 @@ describe CMockGeneratorUtils, "Verify CMockGeneratorUtils Module" do arg3 = { :name => "Kiwi", :const? => false, :type => 'KIWI_T*', :ptr? => true } expected3 = " cmock_call_instance->Expected_Kiwi = Kiwi;\n" + - " cmock_call_instance->Expected_Kiwi_Depth = Kiwi_Depth;\n" + + " cmock_call_instance->Expected_Kiwi_Depth = Mango_Depth;\n" + " cmock_call_instance->IgnoreArg_Kiwi = 0;\n" + " cmock_call_instance->ReturnThruPtr_Kiwi_Used = 0;\n" @@ -141,7 +141,7 @@ describe CMockGeneratorUtils, "Verify CMockGeneratorUtils Module" do assert_equal(expected1, @cmock_generator_utils_complex.code_add_an_arg_expectation(arg1)) assert_equal(expected2, @cmock_generator_utils_complex.code_add_an_arg_expectation(arg2, 'Lemon_Depth')) - assert_equal(expected3, @cmock_generator_utils_complex.code_add_an_arg_expectation(arg3, 'Lemon_Depth')) + assert_equal(expected3, @cmock_generator_utils_complex.code_add_an_arg_expectation(arg3, 'Mango_Depth')) assert_equal(expected4, @cmock_generator_utils_complex.code_add_an_arg_expectation(arg4)) end diff --git a/test/unit/cmock_header_parser_test.rb b/test/unit/cmock_header_parser_test.rb index 4cf1819..d680355 100644 --- a/test/unit/cmock_header_parser_test.rb +++ b/test/unit/cmock_header_parser_test.rb @@ -2879,5 +2879,128 @@ describe CMockHeaderParser, "Verify CMockHeaderParser Module" do assert_equal(expected, @parser.transform_inline_functions(source)) end + it "pairs size arg with adjacent pointer when no name affinity exists" do + source = "void process(int *data, int size)" + + result = @parser.parse("module", source) + args = result[:functions].first[:args] + + data_arg = args.find { |a| a[:name] == 'data' } + size_arg = args.find { |a| a[:name] == 'size' } + + assert_equal(:after, data_arg[:array_size_order], "data should be paired with size (after)") + assert_equal(true, size_arg[:array_size?], "size should be marked as array_size?") + end + + it "pairs size arg with the pointer whose name it matches, even when a different pointer is adjacent" do + # buf_size is adjacent to status, but 'buf' root matches 'buf' exactly + source = "void store(int *status, int buf_size, int *buf)" + + result = @parser.parse("module", source) + args = result[:functions].first[:args] + + status_arg = args.find { |a| a[:name] == 'status' } + buf_size_arg = args.find { |a| a[:name] == 'buf_size' } + buf_arg = args.find { |a| a[:name] == 'buf' } + + assert_nil(status_arg[:array_size_order], "status should NOT be paired") + assert_equal(:before, buf_arg[:array_size_order], "buf should be paired with size (before)") + assert_equal(true, buf_size_arg[:array_size?], "buf_size should be marked as array_size?") + end + + it "pairs size arg with pointer that comes after it when name affinity is stronger than adjacency" do + # buff_size is adjacent to bytes_to_read, but 'buff' root is a prefix of 'buffer' + source = "void read_interface(size_t *bytes_to_read, size_t buff_size, char **buffer)" + + result = @parser.parse("module", source) + args = result[:functions].first[:args] + + bytes_arg = args.find { |a| a[:name] == 'bytes_to_read' } + buff_size_arg = args.find { |a| a[:name] == 'buff_size' } + buffer_arg = args.find { |a| a[:name] == 'buffer' } + + assert_nil(bytes_arg[:array_size_order], "bytes_to_read should NOT be paired") + assert_equal(:before, buffer_arg[:array_size_order], "buffer should be paired with size (before)") + assert_equal(true, buff_size_arg[:array_size?], "buff_size should be marked as array_size?") + end + + it "pairs multiple size args each with their best-matched pointer" do + source = "void copy(int *dst, int dst_len, int *src, int src_len)" + + result = @parser.parse("module", source) + args = result[:functions].first[:args] + + dst_arg = args.find { |a| a[:name] == 'dst' } + dst_len_arg = args.find { |a| a[:name] == 'dst_len' } + src_arg = args.find { |a| a[:name] == 'src' } + src_len_arg = args.find { |a| a[:name] == 'src_len' } + + assert_equal(:after, dst_arg[:array_size_order], "dst should be paired with size (after)") + assert_equal(true, dst_len_arg[:array_size?], "dst_len should be marked as array_size?") + assert_equal(:after, src_arg[:array_size_order], "src should be paired with size (after)") + assert_equal(true, src_len_arg[:array_size?], "src_len should be marked as array_size?") + end + + it "pairs size arg with adjacent array when no name affinity exists" do + source = "void process(int data[], int size)" + + result = @parser.parse("module", source) + args = result[:functions].first[:args] + + data_arg = args.find { |a| a[:name] == 'data' } + size_arg = args.find { |a| a[:name] == 'size' } + + assert_equal(:after, data_arg[:array_size_order], "data should be paired with size (after)") + assert_equal(true, size_arg[:array_size?], "size should be marked as array_size?") + end + + it "pairs size arg with the array whose name it matches, even when a different pointer is adjacent" do + # buf_size is adjacent to status, but 'buf' root matches 'buf' exactly + source = "void store(int status[], int buf_size, int buf[])" + + result = @parser.parse("module", source) + args = result[:functions].first[:args] + + status_arg = args.find { |a| a[:name] == 'status' } + buf_size_arg = args.find { |a| a[:name] == 'buf_size' } + buf_arg = args.find { |a| a[:name] == 'buf' } + + assert_nil(status_arg[:array_size_order], "status should NOT be paired") + assert_equal(:before, buf_arg[:array_size_order], "buf should be paired with size (before)") + assert_equal(true, buf_size_arg[:array_size?], "buf_size should be marked as array_size?") + end + + it "pairs size arg with array that comes after it when name affinity is stronger than adjacency" do + # buff_size is adjacent to bytes_to_read, but 'buff' root is a prefix of 'buffer' + source = "void read_interface(size_t bytes_to_read[], size_t buff_size, char *buffer[])" + + result = @parser.parse("module", source) + args = result[:functions].first[:args] + + bytes_arg = args.find { |a| a[:name] == 'bytes_to_read' } + buff_size_arg = args.find { |a| a[:name] == 'buff_size' } + buffer_arg = args.find { |a| a[:name] == 'buffer' } + + assert_nil(bytes_arg[:array_size_order], "bytes_to_read should NOT be paired") + assert_equal(:before, buffer_arg[:array_size_order], "buffer should be paired with size (before)") + assert_equal(true, buff_size_arg[:array_size?], "buff_size should be marked as array_size?") + end + + it "pairs multiple size args each with their best-matched array" do + source = "void copy(int dst[], int dst_len, int src[], int src_len)" + + result = @parser.parse("module", source) + args = result[:functions].first[:args] + + dst_arg = args.find { |a| a[:name] == 'dst' } + dst_len_arg = args.find { |a| a[:name] == 'dst_len' } + src_arg = args.find { |a| a[:name] == 'src' } + src_len_arg = args.find { |a| a[:name] == 'src_len' } + + assert_equal(:after, dst_arg[:array_size_order], "dst should be paired with size (after)") + assert_equal(true, dst_len_arg[:array_size?], "dst_len should be marked as array_size?") + assert_equal(:after, src_arg[:array_size_order], "src should be paired with size (after)") + assert_equal(true, src_len_arg[:array_size?], "src_len should be marked as array_size?") + end end