fix(ino): never hoist an #include out of its #if (#1443)

* fix(ino): never hoist an #include out of its #if

The .ino -> .ino.cpp conversion (#1275) moved every line
starting with `#include` into the prelude and blanked it in the body,
with no notion of preprocessor conditionals. A guarded include such as

    #if defined(FL_IS_TEENSY)
    #include <Audio.h>
    #endif

was emitted bare above the sketch, so every non-Teensy build of FastLED's
root project (Sailboat) failed with "Audio.h: No such file". The same
scan also reordered a #define ahead of the header it configures, matched
`#include` inside /* */ comments, and missed `#  include`.

Hoist only each tab's leading preprocessor region: the blank lines,
comments and directives before the first line of code, cut back to where
#if nesting is closed.
- The first tab's region moves verbatim and in order (#if blocks,
  #defines and all). It precedes all other code anyway, so semantics are
  unchanged, and headers still precede the prototypes (the #1275 goal).
- Later tabs contribute only their unconditional #includes; their other
  directives stay in place so they cannot change what earlier tabs see.
- Includes after the first line of code stay where they are, as with
  arduino-cli.

Fixes #1440

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

* test(ino): move the #1440 hoisting tests to their own module

tests.rs crossed the 1000-LOC gate; the new cases live in
source_scanner/tests_include_hoisting.rs and reuse setup_project.

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

* fix(ino): keep a continued #include in a later tab whole

A later tab's `#include \\` + `<Wire.h>` hoisted only the continuation
line, leaving a bare `<Wire.h>` in the prelude and a dangling directive in
the body. Mark continuation lines and hoist only single-line includes from
later tabs. Review follow-up.

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

* style: apply formatting

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: zackees

crates/fbuild-build-engine/src/source_scanner.rs

@@ -349,17 +349,14 @@ impl SourceScanner {
             })
             .collect::<fbuild_core::Result<Vec<_>>>()?;
 
-        // Hoist every tab's `#include` directives into the prelude so that
+        // Hoist each tab's leading preprocessor lines into the prelude so that
         // auto-generated prototypes can reference types from library headers
         // (FastLED/fbuild#1275 — arduino-cli places prototypes *after* the
-        // include block for exactly this reason).  Replace each hoisted
-        // `#include` line with a blank line in the body to preserve line
-        // numbering for diagnostics.
-        let sketch_includes = hoist_include_directives(&contents);
-        let stripped_contents: Vec<String> = contents
-            .iter()
-            .map(|c| strip_include_directives(c))
-            .collect();
+        // include block for exactly this reason), without lifting an
+        // `#include` out of its `#if` (FastLED/fbuild#1440). Hoisted lines
+        // become blank lines in the body to preserve line numbering for
+        // diagnostics.
+        let (sketch_includes, stripped_contents) = hoist_leading_preprocessor(&contents);
 
         // Prototype extraction needs to see every tab's code, so it operates
         // on the plain concatenation (no #line noise).  We feed it the
@@ -787,44 +784,176 @@ fn walk_sources(dir: &Path) -> Vec<PathBuf> {
     files
 }
 
-/// Extract every `#include` directive across all tabs, deduplicated and in
-/// first-seen order, for hoisting into the prelude (FastLED/fbuild#1275).
-fn hoist_include_directives(contents: &[String]) -> Vec<String> {
-    let mut seen = HashSet::new();
-    let mut includes = Vec::new();
-    for content in contents {
-        for line in content.lines() {
-            let trimmed = line.trim();
-            if is_include_directive(trimmed) {
-                let normalized = trimmed.to_string();
-                if seen.insert(normalized.clone()) {
-                    includes.push(normalized);
+/// One line of a tab's leading preprocessor region.
+struct LeadingLine {
+    index: usize,
+    /// `#if` nesting depth the line sits at (0 = unconditional).
+    depth: usize,
+    /// Part of an `#include` directive.
+    is_include: bool,
+    /// A `\`-continuation of the directive on a previous line.
+    is_continuation: bool,
+}
+
+/// The lines before a tab's first line of code -- blank lines, comments and
+/// preprocessor directives (with `\` continuations) -- cut back to the last
+/// point where `#if` nesting is closed, so a region never ends inside a
+/// conditional block.
+fn leading_preprocessor_region(source: &str) -> Vec<LeadingLine> {
+    let mut lines = Vec::new();
+    let mut depth = 0usize;
+    let mut in_block_comment = false;
+    let mut continuation: Option<(usize, bool)> = None;
+    let mut boundary = 0usize;
+
+    for (index, line) in source.lines().enumerate() {
+        let trimmed = line.trim();
+        if let Some((cont_depth, cont_include)) = continuation {
+            lines.push(LeadingLine {
+                index,
+                depth: cont_depth,
+                is_include: cont_include,
+                is_continuation: true,
+            });
+            if !trimmed.ends_with('\\') {
+                continuation = None;
+            }
+        } else if in_block_comment {
+            if let Some(end) = trimmed.find("*/") {
+                if !is_comment_tail(&trimmed[end + 2..]) {
+                    break;
+                }
+                in_block_comment = false;
+            }
+            lines.push(LeadingLine {
+                index,
+                depth,
+                is_include: false,
+                is_continuation: false,
+            });
+        } else if trimmed.is_empty() || trimmed.starts_with("//") {
+            lines.push(LeadingLine {
+                index,
+                depth,
+                is_include: false,
+                is_continuation: false,
+            });
+        } else if let Some(after_open) = trimmed.strip_prefix("/*") {
+            match after_open.find("*/") {
+                Some(end) if !is_comment_tail(&after_open[end + 2..]) => break,
+                Some(_) => {}
+                None => in_block_comment = true,
+            }
+            lines.push(LeadingLine {
+                index,
+                depth,
+                is_include: false,
+                is_continuation: false,
+            });
+        } else if let Some(directive) = trimmed.strip_prefix('#') {
+            let name: String = directive
+                .trim_start()
+                .chars()
+                .take_while(|c| c.is_ascii_alphanumeric() || *c == '_')
+                .collect();
+            let line_depth = match name.as_str() {
+                "if" | "ifdef" | "ifndef" => {
+                    depth += 1;
+                    depth - 1
+                }
+                "elif" | "else" => depth.saturating_sub(1),
+                "endif" => {
+                    if depth == 0 {
+                        break;
+                    }
+                    depth -= 1;
+                    depth
                 }
+                _ => depth,
+            };
+            let is_include = name == "include";
+            lines.push(LeadingLine {
+                index,
+                depth: line_depth,
+                is_include,
+                is_continuation: false,
+            });
+            if trimmed.ends_with('\\') {
+                continuation = Some((line_depth, is_include));
             }
+        } else {
+            break;
+        }
+        if depth == 0 && continuation.is_none() && !in_block_comment {
+            boundary = lines.len();
         }
     }
-    includes
+    lines.truncate(boundary);
+    lines
 }
 
-/// Replace every `#include` line with an empty line so that line numbering
-/// is preserved when the hoisted directives are moved to the prelude
-/// (FastLED/fbuild#1275).
-fn strip_include_directives(source: &str) -> String {
-    source
-        .lines()
-        .map(|line| {
-            if is_include_directive(line.trim()) {
-                ""
-            } else {
-                line
-            }
-        })
-        .collect::<Vec<_>>()
-        .join("\n")
+/// True when what follows a closing `*/` on the same line is nothing but
+/// whitespace or a `//` comment.
+fn is_comment_tail(rest: &str) -> bool {
+    let rest = rest.trim();
+    rest.is_empty() || rest.starts_with("//")
 }
 
-fn is_include_directive(trimmed: &str) -> bool {
-    trimmed.starts_with("#include")
+/// Move each tab's leading preprocessor lines into the prelude so the
+/// auto-generated prototypes, which sit in the prelude, can use types from
+/// the sketch's headers (FastLED/fbuild#1275). Moved lines become blank lines
+/// in the body so diagnostics keep their line numbers.
+///
+/// Only lines *before the first line of code* move, and an `#include` never
+/// leaves its conditional (FastLED/fbuild#1440): a Teensy-only
+/// `#if ... #include <Audio.h> #endif` used to be hoisted bare and compiled
+/// on every board.
+///
+/// - The first tab's region moves verbatim and in order -- includes, `#if`
+///   blocks, `#define`s and comments. It precedes all other code anyway, so a
+///   `#define` that configures a later header still precedes that header.
+/// - Later tabs contribute only their unconditional `#include`s. Their other
+///   directives stay put: moving a later tab's `#define` above the first
+///   tab's code would change what that code sees.
+///
+/// Unconditional includes are de-duplicated by trimmed text across tabs.
+/// Returns the hoisted lines and each tab's body with those lines blanked.
+fn hoist_leading_preprocessor(contents: &[String]) -> (Vec<String>, Vec<String>) {
+    let mut seen_includes = HashSet::new();
+    let mut hoisted = Vec::new();
+    let mut stripped = Vec::with_capacity(contents.len());
+
+    for (tab, content) in contents.iter().enumerate() {
+        let source_lines: Vec<&str> = content.lines().collect();
+        let mut moved = HashSet::new();
+        for leading in leading_preprocessor_region(content) {
+            let line = source_lines[leading.index];
+            let unconditional_include = leading.is_include && leading.depth == 0;
+            if tab == 0 {
+                if unconditional_include {
+                    seen_includes.insert(line.trim().to_string());
+                }
+                hoisted.push(line.to_string());
+                moved.insert(leading.index);
+            } else if unconditional_include
+                && !leading.is_continuation
+                && !line.trim_end().ends_with('\\')
+            {
+                if seen_includes.insert(line.trim().to_string()) {
+                    hoisted.push(line.trim().to_string());
+                }
+                moved.insert(leading.index);
+            }
+        }
+        let body = source_lines
+            .iter()
+            .enumerate()
+            .map(|(i, line)| if moved.contains(&i) { "" } else { *line })
+            .collect::<Vec<_>>()
+            .join("\n");
+        stripped.push(body);
+    }
+    (hoisted, stripped)
 }
 
 /// Extract function prototypes from concatenated .ino source using a C++ parser.
@@ -1220,3 +1349,6 @@ fn trim_trailing_spaces(text: &mut String) {
 
 #[cfg(test)]
 mod tests;
+
+#[cfg(test)]
+mod tests_include_hoisting;

crates/fbuild-build-engine/src/source_scanner/tests.rs

@@ -5,7 +5,7 @@ use super::*;
 use std::fs;
 use tempfile::TempDir;
 
-fn setup_project(src_files: &[(&str, &str)]) -> (TempDir, PathBuf, PathBuf) {
+pub(super) fn setup_project(src_files: &[(&str, &str)]) -> (TempDir, PathBuf, PathBuf) {
     let tmp = TempDir::new().unwrap();
     let src_dir = tmp.path().join("src");
     let build_dir = tmp.path().join("build");

crates/fbuild-build-engine/src/source_scanner/tests_include_hoisting.rs

@@ -0,0 +1,143 @@
+//! FastLED/fbuild#1440: `.ino` include hoisting must never lift an `#include`
+//! out of its `#if`. Split from `tests.rs` to keep it under the 1000-LOC gate.
+
+use super::tests::setup_project;
+use super::*;
+use std::fs;
+
+/// Generated `.ino.cpp` for a single-tab sketch, split at its `#line 1`.
+fn generated_prelude_and_body(sketch: &str) -> (String, String) {
+    let (_tmp, src_dir, build_dir) = setup_project(&[("sketch.ino", sketch)]);
+    let scanner = SourceScanner::new(&src_dir, &build_dir);
+    let sources = scanner.scan_sketch_sources().unwrap();
+    let content = fs::read_to_string(&sources[0]).unwrap();
+    let (prelude, body) = content
+        .split_once("#line 1 \"src/sketch.ino\"\n")
+        .expect("generated file carries a #line 1 directive");
+    (prelude.to_string(), body.to_string())
+}
+
+#[test]
+fn test_conditional_include_stays_inside_its_if() {
+    // The FastLED Sailboat shape that broke every non-Teensy root build.
+    let (prelude, body) = generated_prelude_and_body(
+        "#include <FastLED.h>\n\n#if defined(FL_IS_TEENSY)\n#include <Audio.h>\n#endif\n#include \"fl/ui/ui.h\"\n\nint helper(int x) { return x; }\nvoid setup() {}\nvoid loop() {}\n",
+    );
+    let lines: Vec<&str> = prelude.lines().collect();
+    let if_pos = lines
+        .iter()
+        .position(|l| *l == "#if defined(FL_IS_TEENSY)")
+        .expect("the #if moves with its include");
+    assert_eq!(lines[if_pos + 1], "#include <Audio.h>");
+    assert_eq!(lines[if_pos + 2], "#endif");
+    // Exactly one Audio.h, and only inside the guard.
+    assert_eq!(prelude.matches("#include <Audio.h>").count(), 1);
+    assert!(!body.contains("#include <Audio.h>"));
+    // Headers after the block still precede the prototypes.
+    let ui_pos = prelude.find("#include \"fl/ui/ui.h\"").unwrap();
+    let proto_pos = prelude
+        .find("// Auto-generated function prototypes")
+        .unwrap();
+    assert!(ui_pos < proto_pos);
+}
+
+#[test]
+fn test_define_before_include_keeps_its_order() {
+    let (prelude, _body) = generated_prelude_and_body(
+        "#define FASTLED_CONFIG_KNOB 1\n#include <FastLED.h>\n\nvoid setup() {}\nvoid loop() {}\n",
+    );
+    let define_pos = prelude.find("#define FASTLED_CONFIG_KNOB 1").unwrap();
+    let include_pos = prelude.find("#include <FastLED.h>").unwrap();
+    assert!(
+        define_pos < include_pos,
+        "a #define that configures a header must still precede it"
+    );
+}
+
+#[test]
+fn test_include_after_first_code_line_stays_in_place() {
+    let (prelude, body) = generated_prelude_and_body(
+        "#include <FastLED.h>\nint counter = 0;\n#include \"late.h\"\n\nvoid setup() {}\nvoid loop() {}\n",
+    );
+    assert!(!prelude.contains("late.h"));
+    let body_lines: Vec<&str> = body.lines().collect();
+    assert_eq!(body_lines[0], "", "the leading include is blanked");
+    assert_eq!(body_lines[1], "int counter = 0;");
+    assert_eq!(body_lines[2], "#include \"late.h\"", "left on its own line");
+}
+
+#[test]
+fn test_spaced_include_directive_is_recognised() {
+    let (prelude, body) =
+        generated_prelude_and_body("#  include <FastLED.h>\nvoid setup() {}\nvoid loop() {}\n");
+    assert!(prelude.contains("#  include <FastLED.h>"));
+    assert!(!body.contains("include <FastLED.h>"));
+}
+
+#[test]
+fn test_include_inside_block_comment_is_not_a_directive() {
+    let (prelude, body) = generated_prelude_and_body(
+        "/*\n#include <Nope.h>\n*/\n#include <FastLED.h>\nvoid setup() {}\nvoid loop() {}\n",
+    );
+    // The comment moves verbatim, so the commented-out include is still
+    // inside a comment -- never a bare directive.
+    let comment_open = prelude.find("/*").unwrap();
+    let nope = prelude.find("#include <Nope.h>").unwrap();
+    let comment_close = prelude.find("*/").unwrap();
+    assert!(comment_open < nope && nope < comment_close);
+    assert!(!body.contains("Nope.h"));
+}
+
+#[test]
+fn test_later_tab_hoists_only_unconditional_includes() {
+    let (_tmp, src_dir, build_dir) = setup_project(&[
+        (
+            "main.ino",
+            "#include <FastLED.h>\nvoid setup() {}\nvoid loop() {}\n",
+        ),
+        (
+            "tab.ino",
+            "#define TAB_ONLY 1\n#include <Wire.h>\n#ifdef ARDUINO_TEENSY41\n#include <Audio.h>\n#endif\nvoid helper() {}\n",
+        ),
+    ]);
+    let scanner = SourceScanner::new(&src_dir, &build_dir);
+    let sources = scanner.scan_sketch_sources().unwrap();
+    let content = fs::read_to_string(&sources[0]).unwrap();
+    let (prelude, rest) = content.split_once("#line 1 \"src/main.ino\"\n").unwrap();
+    let (_, tab_body) = rest.split_once("#line 1 \"src/tab.ino\"\n").unwrap();
+
+    assert!(prelude.contains("#include <Wire.h>"));
+    // A later tab's #define and guarded include stay in that tab, in order.
+    assert!(!prelude.contains("TAB_ONLY"));
+    assert!(!prelude.contains("Audio.h"));
+    let tab_lines: Vec<&str> = tab_body.lines().collect();
+    assert_eq!(tab_lines[0], "#define TAB_ONLY 1");
+    assert_eq!(tab_lines[1], "", "the unconditional include is blanked");
+    assert_eq!(tab_lines[2], "#ifdef ARDUINO_TEENSY41");
+    assert_eq!(tab_lines[3], "#include <Audio.h>");
+    assert_eq!(tab_lines[4], "#endif");
+}
+
+#[test]
+fn test_later_tab_continued_include_stays_whole() {
+    // A `\`-continued include in a later tab must not be split: hoisting its
+    // final line alone left a bare `<Wire.h>` in the prelude and a dangling
+    // `#include \` in the body.
+    let (_tmp, src_dir, build_dir) = setup_project(&[
+        (
+            "main.ino",
+            "#include <FastLED.h>\nvoid setup() {}\nvoid loop() {}\n",
+        ),
+        ("tab.ino", "#include \\\n<Wire.h>\nvoid helper() {}\n"),
+    ]);
+    let scanner = SourceScanner::new(&src_dir, &build_dir);
+    let sources = scanner.scan_sketch_sources().unwrap();
+    let content = fs::read_to_string(&sources[0]).unwrap();
+    let (prelude, rest) = content.split_once("#line 1 \"src/main.ino\"\n").unwrap();
+    let (_, tab_body) = rest.split_once("#line 1 \"src/tab.ino\"\n").unwrap();
+
+    assert!(!prelude.contains("Wire.h"));
+    let tab_lines: Vec<&str> = tab_body.lines().collect();
+    assert_eq!(tab_lines[0], "#include \\");
+    assert_eq!(tab_lines[1], "<Wire.h>");
+}