Fix Partials merging adjacent #define macros when one has a comment with an apostrophe (#1262)

docs/Changelog.md

@@ -14,6 +14,10 @@ This changelog is complemented by three other documents:
 
 ## 💪 Fixed
 
+### Partials
+
+- [#1262](https://github.com/ThrowTheSwitch/Ceedling/issues/1262) Fixed a Partials-generated header occasionally concatenating two adjacent `#define` lines in one line causing a stray '#' compilation error. The triggering condition involved a `//` or `/* */` comment containing an apostrophe or quote (e.g. `// Don't ...`) that disrupted string literal handling.
+
 ### Gcov plugin
 
 - Fixed `:gcov ↳ :gcovr ↳ :decisions` reporting an incorrect minimum-version requirement; the version check (and docs) now correctly require `gcovr` 5.1 or higher.

lib/ceedling/c_extractor/c_extractor_preprocessing.rb

@@ -241,9 +241,46 @@ def _collect_directive(scanner)
     text = scanner.scan(/#/)
 
     until scanner.eos?
-      text << (scanner.scan(/[^"'\\\n]*/) || '')
+      # '/' is excluded here (alongside the quote/backslash/newline characters
+      # already excluded) so a comment marker is never swallowed into this plain
+      # literal run -- it has to be seen and dispatched on its own below, before
+      # any '"' or "'" inside the comment's own text gets mistaken for the start
+      # of a string/char literal (see the // and /* branches for why that matters).
+      text << (scanner.scan(/[^"'\\\n\/]*/) || '')
+
+      if scanner.scan(%r{//})
+        # A trailing line comment (e.g. "// don't change this") can contain an
+        # apostrophe or quote that isn't a literal delimiter at all. Consumed the
+        # ordinary way, that stray quote would send skip_c_string hunting for a
+        # closing match, straight through this directive's own newline and into
+        # whatever follows -- silently merging the next #define into this one.
+        # Comment content is captured verbatim and never scanned for literals.
+        #
+        # A '\' immediately before the physical newline still splices lines here,
+        # same as everywhere else in a directive (line splicing is a translation
+        # phase that happens before comments are stripped), so the comment -- and
+        # the directive -- keeps going onto the next physical line rather than
+        # ending at that newline.
+        before = scanner.pos - 2
+        loop do
+          scanner.scan(/[^\\\n]*/)
+          break if scanner.eos?
+          break unless scanner.scan(/\\\n/) || scanner.scan(/\\/)
+        end
+        text << scanner.string[before...scanner.pos]
+
+      elsif scanner.scan(%r{/\*})
+        # A block comment can legitimately span physical lines without ending the
+        # directive (only an un-commented newline does that) -- its content, quotes
+        # included, is likewise never scanned for literals, only for its own close.
+        before = scanner.pos - 2
+        scanner.skip_until(%r{\*/}) || scanner.terminate
+        text << scanner.string[before...scanner.pos]
+
+      elsif scanner.scan(%r{/})
+        text << '/'
 
-      if (ch = scanner.peek(1)) == '"' || ch == "'"
+      elsif (ch = scanner.peek(1)) == '"' || ch == "'"
         before = scanner.pos
         @c_extractor_code_text.skip_c_string(scanner, ch)
         text << scanner.string[before...scanner.pos]

spec/units/c_extractor/c_extractor_integration_spec.rb

@@ -565,6 +565,59 @@
       expect( contents.variable_declarations.length ).to eq 0
     end
 
+    # #1262: Partials-generated headers occasionally concatenated two adjacent
+    # #define lines onto one physical line. Reconstructing the joined text
+    # from element_sequence (as generator_partials.rb does) rules the plain,
+    # comment-free case in or out formally, rather than by inspection of
+    # #_collect_directive alone. This shape -- three plain register-offset
+    # macros in a row, no comments -- is expected to reconstruct cleanly.
+    it "reconstructs three adjacent plain #define macros joined by newlines with no text lost between them" do
+      file_contents = <<~'CONTENTS'
+      #define START_ADDRESS 0x00
+      #define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)
+      #define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)
+      CONTENTS
+
+      contents = extract_from.call(file_contents)
+
+      expect( contents.macro_definitions.length ).to eq 3
+      joined = contents.element_sequence.map(&:text).join("\n")
+      expect( joined ).to eq(
+        "#define START_ADDRESS 0x00\n" +
+        "#define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)\n" +
+        "#define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)"
+      )
+    end
+
+    # #1262's actual reported trigger: a trailing // comment on one #define containing
+    # an ordinary English contraction. The stray apostrophe used to be scanned as the
+    # start of a char literal, sending the directive scanner hunting for a closing
+    # match straight through this macro's own newline and into the next two #defines
+    # -- merging all three into one macro_definitions entry with no separating newline,
+    # which is exactly what later caused Partials-generated headers to concatenate two
+    # #define lines onto one physical line ("stray '#' in program").
+    it "reconstructs three adjacent #define macros when one has a trailing comment with an apostrophe" do
+      file_contents = <<~'CONTENTS'
+      #define START_ADDRESS 0x00 // don't change this
+      #define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)
+      #define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)
+      CONTENTS
+
+      contents = extract_from.call(file_contents)
+
+      expect( contents.macro_definitions.length ).to eq 3
+      expect( contents.macro_definitions[0].text ).to eq "#define START_ADDRESS 0x00 // don't change this"
+      expect( contents.macro_definitions[1].text ).to eq "#define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)"
+      expect( contents.macro_definitions[2].text ).to eq "#define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)"
+
+      joined = contents.element_sequence.map(&:text).join("\n")
+      expect( joined ).to eq(
+        "#define START_ADDRESS 0x00 // don't change this\n" +
+        "#define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)\n" +
+        "#define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)"
+      )
+    end
+
     it "should extract a function definition following a #define with an escaped character in a string literal (GH #1184)" do
       file_contents = <<~'CONTENTS'
       #include "world.h"

spec/units/c_extractor/c_extractor_preprocessing_spec.rb

@@ -336,6 +336,51 @@ def try_directive(text)
       expect(pos).to eq "#define FOO 1\n".length
     end
 
+    # --- Regression: GH #1262 — a trailing // comment's apostrophe/quote merges
+    # the next directive into this one ---
+    # A '\'' or '"' inside an ordinary // comment (e.g. an English contraction
+    # like "don't") is not a string/char literal delimiter. Scanned as one
+    # anyway, it sends skip_c_string hunting for a closing match straight through
+    # this directive's own newline and into whatever follows, silently merging
+    # the next #define's text (and losing the newline between them) into this one.
+
+    it "extracts a #define with a trailing // comment containing an apostrophe" do
+      input = "#define START_ADDRESS 0x00 // don't change this\n"
+      result, pos = try_directive(input)
+      expect(result).to eq [true, input.rstrip]
+      expect(pos).to eq input.length
+    end
+
+    it "does not consume a following #define after a // comment containing an apostrophe" do
+      input = "#define START_ADDRESS 0x00 // don't change this\n#define IDX 1\n"
+      result, pos = try_directive(input)
+      expect(result).to eq [true, "#define START_ADDRESS 0x00 // don't change this"]
+      expect(pos).to eq "#define START_ADDRESS 0x00 // don't change this\n".length
+    end
+
+    it "does not consume a following #define after a // comment containing a double quote" do
+      input = %(#define LABEL "unbalanced) + %( quote in a comment: "\n#define IDX 1\n)
+      result, pos = try_directive(input)
+      expect(result[0]).to eq true
+      expect(result[1]).to start_with('#define LABEL')
+      expect(pos).to be < input.length
+      expect(input[pos..]).to eq "#define IDX 1\n"
+    end
+
+    it "extracts a #define with a trailing /* */ comment containing an apostrophe" do
+      input = "#define START_ADDRESS 0x00 /* don't change this */\n"
+      result, pos = try_directive(input)
+      expect(result).to eq [true, input.rstrip]
+      expect(pos).to eq input.length
+    end
+
+    it "carries a directive across physical lines when a // comment's own line ends in a backslash" do
+      input = "#define MACRO(x) \\\n  // comment continues \\\n  do_thing(x);\n"
+      result, pos = try_directive(input)
+      expect(result).to eq [true, input.rstrip]
+      expect(pos).to eq input.length
+    end
+
     it "leaves scanner position unchanged on failure" do
       scanner = StringScanner.new("int x;")
       scanner.pos = 0

spec/units/generators/generator_partials_spec.rb

@@ -397,6 +397,56 @@ def empty_module
 
       expect( buf.string ).to_not include("#define FOO 1")
     end
+
+    # #1262: two consecutive typedefs (no macro carry-forward involved at
+    # all) had no byte-exact adjacency coverage -- only the macro-before-
+    # typedef pending_macros path did.
+    it "emits two consecutive typedefs each on their own line" do
+      output_path = '/path/to/output'
+      name = 'my_module'
+      header_filename = 'my_module_types.h'
+
+      allow(@file_path_utils).to receive(:form_partial_types_header_filename).and_return(header_filename)
+
+      buf = StringIO.new()
+      allow(@file_wrapper).to receive(:open).and_yield(buf)
+
+      typedef_a = CExtractorTypes::CStatement.new(text: "typedef uint8_t Byte;", line_num: 1)
+      typedef_b = CExtractorTypes::CStatement.new(text: "typedef uint16_t Word;", line_num: 2)
+
+      c_module = CExtractorTypes::CModule.new(
+        type_definitions: [typedef_a, typedef_b],
+        element_sequence: [typedef_a, typedef_b]
+      )
+
+      @generator.generate_types(name: name, c_module: c_module, output_path: output_path)
+
+      expect( buf.string ).to include( "typedef uint8_t Byte;\ntypedef uint16_t Word;\n" )
+    end
+
+    # #1262: same gap for two consecutive aggregate definitions.
+    it "emits two consecutive aggregate definitions each on their own line" do
+      output_path = '/path/to/output'
+      name = 'my_module'
+      header_filename = 'my_module_types.h'
+
+      allow(@file_path_utils).to receive(:form_partial_types_header_filename).and_return(header_filename)
+
+      buf = StringIO.new()
+      allow(@file_wrapper).to receive(:open).and_yield(buf)
+
+      aggregate_a = CExtractorTypes::CStatement.new(text: "struct Point { int x; int y; };", line_num: 1)
+      aggregate_b = CExtractorTypes::CStatement.new(text: "struct Color { int r; int g; int b; };", line_num: 2)
+
+      c_module = CExtractorTypes::CModule.new(
+        aggregate_definitions: [aggregate_a, aggregate_b],
+        element_sequence:      [aggregate_a, aggregate_b]
+      )
+
+      @generator.generate_types(name: name, c_module: c_module, output_path: output_path)
+
+      expect( buf.string ).to include( "struct Point { int x; int y; };\nstruct Color { int r; int g; int b; };\n" )
+    end
   end
 
   context "#generate_header (private method)" do
@@ -555,6 +605,54 @@ def empty_module
       expect( buf.string.strip() ).to eq file_contents.strip()
     end
 
+    # #1262: a header with several single-line macros in a row and nothing
+    # type-defining after them (so generate_types never runs at all, per its
+    # own empty-module guard) is the ordinary, ungoverned case -- every macro
+    # here goes through this inline CStatement branch, not generate_types'
+    # own carry-forward logic, which already had its own adjacency coverage.
+    it "emits three consecutive macros each on their own line, with nothing following them" do
+      file_contents = <<~CONTENTS
+      #ifndef __CEEDLING_GENERATED_REGS_H__
+      #define __CEEDLING_GENERATED_REGS_H__
+
+      #define START_ADDRESS 0x00
+      #define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)
+      #define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)
+
+      #endif // __CEEDLING_GENERATED_REGS_H__
+
+      CONTENTS
+
+      c_module = make_module(
+        make_stmt(text: "#define START_ADDRESS 0x00", line_num: 1),
+        make_stmt(text: "#define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)", line_num: 2),
+        make_stmt(text: "#define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)", line_num: 3)
+      )
+
+      @generator.send(:generate_header, buf, 'regs', [], [], c_module, false)
+      expect( buf.string.strip() ).to eq file_contents.strip()
+    end
+
+    # #1262 regression: the actual reported shape -- one macro's trailing // comment
+    # includes an apostrophe. Extraction is what previously merged the macros (see
+    # c_extractor specs); this confirms generate_header still emits each item's text,
+    # comment included, on its own line once extraction is correct.
+    it "emits each macro on its own line even when one has a trailing comment with an apostrophe (GH #1262)" do
+      c_module = make_module(
+        make_stmt(text: "#define START_ADDRESS 0x00 // don't change this", line_num: 1),
+        make_stmt(text: "#define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)", line_num: 2),
+        make_stmt(text: "#define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)", line_num: 3)
+      )
+
+      @generator.send(:generate_header, buf, 'regs', [], [], c_module, false)
+
+      expect( buf.string ).to include(
+        "#define START_ADDRESS 0x00 // don't change this\n" +
+        "#define IDX_DATARATE (ADS124S08_REG_ADDR_DATARATE - START_ADDRESS)\n" +
+        "#define IDX_REF (ADS124S08_REG_ADDR_REF - START_ADDRESS)\n"
+      )
+    end
+
     it "should emit macro and variable statements inline while routing typedefs and aggregates to the shared types header" do
       # One item of each category that can appear in a generated Partial header:
       #   macro_definitions     → CStatement emitted as-is, inline