diff --git a/src/lib.rs b/src/lib.rs index b23292c..cec0c04 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -128,14 +128,14 @@ fn blank_line_before( return None; } - // One parser for the two "#" lines that mean "this comment is not explaining - // me". Splitting them was how the BOM strip below ended up on only one of - // the pair, which left a BOM'd file whose first line is "#ifndef GUARD" - // failing a test its BOM-free twin passes. U+FEFF is not Unicode - // White_Space, so "trim" leaves it glued to the "#". + // One parser for the two "#" lines that mean "this comment is not + // explaining me". Splitting them was how the BOM strip below ended up on + // only one of the pair, which left a BOM'd file whose first line is + // "#ifndef GUARD" failing a test its BOM-free twin passes. U+FEFF is not + // Unicode White_Space, so "trim" leaves it glued to the "#". // - // - "#!" on line 1 of a SHELL file is the file's preamble. The header below it - // belongs flush against it, and without this the rule fires on + // - "#!" on line 1 of a SHELL file is the file's preamble. The header + // below it belongs flush against it, and without this the rule fires on // essentially every shell script in existence, detaching its header from // line 1 and splitting a "#! nix-shell -i bash" run off the shebang it // belongs to, which some interpreters require. Gated on "prev_start == 0" @@ -152,7 +152,7 @@ fn blank_line_before( { // "#!" takes "rest" untrimmed: a shebang is the two bytes "#!" with // nothing between them, so "# !x" is an ordinary comment. The - // directives take the trimmed form, because "# if" is valid cpp. + // directives take the trimmed form, because "# if" is valid cpp. // // Shell only, and that gate is load-bearing rather than tidiness: // "#![no_std]" is a Rust inner attribute in exactly the same position, diff --git a/src/normalize.rs b/src/normalize.rs index 91661e1..2b14a89 100644 --- a/src/normalize.rs +++ b/src/normalize.rs @@ -1,6 +1,6 @@ use crate::classify::{DocFlavor, Kind, Style}; pub use crate::linekind::LineKind; -use crate::linekind::{StrippedLine, classify_lines}; +use crate::linekind::{StrippedLine, classify_lines, doxy_tag}; use crate::parse::{Comment, Language}; use crate::textline::{ BookendKind, FAST_PATH_TAB_WIDTH, advance_col, block_is_doc, bookend_match, line_is_art_only, @@ -704,6 +704,60 @@ fn is_foreign_tag(word: &str) -> bool { }) } +/// Whether a whitespace-delimited word is a cross-reference to one of this +/// comment's own parameters rather than a Doxygen block tag. Doxygen prose +/// writes a parameter reference as "@name" ("aligned to @align, or NULL"), the +/// same spelling a block tag uses, so two things separate them. +/// +/// The name has to be one the comment declares, and it must not be a Doxygen +/// tag keyword in its own right: a function with a parameter named "file" or +/// "note", both ordinary in C and both in "DOXY_TAGS", would otherwise swallow +/// a real "@file"/"@note" section into the preceding parameter's description +/// instead of aborting the conversion. +/// +/// Deliberately position-blind. Line position looks like the sharper +/// discriminator (a block tag opens a line, a reference in running prose +/// essentially never does) and it is not, because reflow OWNS line position. +/// In a comment this aborts on, the next run re-wraps the untouched text and +/// merges the line-leading "@file" into the line above; a position test then +/// reads it mid-line as a reference and converts what the run before refused. +/// That is a two-pass fixed point, which "--check" converges on instead of +/// reporting. A name-only test cannot move under the packer. +/// +/// Ceiling: the test is "DOXY_TAGS" membership, and that table is the subset +/// of Doxygen commands this tool knows, not all of them. A parameter named +/// after a command outside it ("@param arg" beside an "@arg" list) still reads +/// the command as a reference and folds it into the description. Widen +/// "DOXY_TAGS" when a real file needs it. +fn is_param_xref(word: &str, param_names: &[&str]) -> bool { + kdoc_xref_name(word).is_some_and(|name| param_names.contains(&name) && doxy_tag(name).is_none()) +} + +/// The parameter name an "@name"/"\\name" cross-reference points at, or "None" if the +/// word cannot be one. Trailing punctuation is trimmed, since a reference at a +/// clause break ("@align,") is still a reference. +/// +/// Both Doxygen spellings are supported, except one-character C escapes such +/// as "\\n"; the name must otherwise open like a C identifier, so "\\0" +/// cannot be a reference. +fn kdoc_xref_name(word: &str) -> Option<&str> { + let kw = word.strip_prefix(['@', '\\'])?; + let name = kw.trim_end_matches(|c: char| !c.is_ascii_alphanumeric() && c != '_'); + + // After the trim, not before: prose ends a clause, and "\\n." is the same + // escape as "\\n". Checking the untrimmed word let a trailing period decide + // whether a comment converted at all. + if word.starts_with('\\') && matches!(name, "a" | "b" | "e" | "f" | "n" | "r" | "t" | "v") { + return None; + } + let starts_like_ident = name + .chars() + .next() + .is_some_and(|c| c.is_ascii_alphabetic() || c == '_'); + (starts_like_ident && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')) + .then_some(name) +} + /// Split a prose line at a mid-line Doxygen return tag, using the very splitter /// reflow uses so the two cannot disagree. /// @@ -783,17 +837,6 @@ fn convert_kernel_doc(lines: Vec, lang: Language) -> Vec { return lines; } - // A whitespace-delimited tag token we don't convert, ANYWHERE in the - // comment (head included), aborts the conversion. Scanning the whole - // comment, not just the tag region, keeps a "@brief"/"@note" in the leading - // description from being silently preserved beside converted params. - let has_foreign_tag = lines - .iter() - .any(|l| l.text.split_whitespace().any(is_foreign_tag)); - if has_foreign_tag { - return lines; - } - // The region opens at the first line that BEGINS with a tag. A tag that // only appears mid-line marks prose describing the tag, not a doc block. let Some(first) = lines.iter().position(|l| { @@ -805,6 +848,46 @@ fn convert_kernel_doc(lines: Vec, lang: Language) -> Vec { return lines; }; + // A whitespace-delimited tag token we don't convert, ANYWHERE in the + // comment (head included), aborts the conversion. Scanning the whole + // comment, not just the tag region, keeps a "@brief"/"@note" in the leading + // description from being silently preserved beside converted params. ... + // except a reference to one of this comment's OWN params. Doxygen prose + // says "a multiple of @align", and kernel-doc keeps that spelling, so + // treating it as a foreign tag aborted every function comment that + // cross-references its arguments (the common case, not a corner). Collect + // the declared names first, then let those words ride along as description + // text. Names come from the tag region ONLY, the same first-word rule that + // opens it: head prose saying "pass @param note to the logger" declares + // nothing. + // + // The scan reads the region's words as ONE stream rather than line by line, + // because that is how the entry scan below reads a name: a "@param" ending + // a line takes its name off the line after. Restarting per line missed that + // name, so the comment aborted, and the next run (reflow having rejoined + // the two) converted it: a two-pass fixed point. + let has_foreign_tag = { + let mut param_names: Vec<&str> = Vec::new(); + let mut words = lines[first..] + .iter() + .flat_map(|l| l.text.split_whitespace()); + while let Some(w) = words.next() { + if matches!(kdoc_tag_of(w), Some(KTag::Param)) + && let Some(n) = words.next().filter(|n| is_kdoc_name(n)) + { + param_names.push(n); + } + } + lines.iter().any(|l| { + l.text + .split_whitespace() + .any(|w| is_foreign_tag(w) && !is_param_xref(w, ¶m_names)) + }) + }; + if has_foreign_tag { + return lines; + } + // Reduce the tag region (first tag line to end) to (tag, description-words) // entries. "bail" keeps "lines" intact so we can return it untouched. let mut entries: Vec<(KTag, Vec)> = Vec::new(); @@ -844,8 +927,15 @@ fn convert_kernel_doc(lines: Vec, lang: Language) -> Vec { at_line_start = false; } } + + // A param name that is itself a convertible tag keyword cannot round-trip: + // "@param return desc" emits "@return : desc", which the NEXT run reads as + // a return tag and rewrites again to "Return : desc". Pass through instead. let bad_param = entries.iter().any(|(tag, desc)| { - matches!(tag, KTag::Param) && !desc.first().is_some_and(|n| is_kdoc_name(n)) + matches!(tag, KTag::Param) + && !desc + .first() + .is_some_and(|n| is_kdoc_name(n) && kdoc_tag_of_keyword(n).is_none()) }); if bail || bad_param { return lines; @@ -1241,6 +1331,156 @@ mod tests { assert_eq!(convert_bodies(lines), vec!["@x : the x", "", "Return zero"]); } + #[test] + fn kdoc_xref_name_forms() { + assert_eq!(kdoc_xref_name("@align"), Some("align")); + assert_eq!(kdoc_xref_name("\\align"), Some("align")); + // Trailing punctuation at a clause break does not hide the reference. + assert_eq!(kdoc_xref_name("@align,"), Some("align")); + assert_eq!(kdoc_xref_name("@size."), Some("size")); + assert_eq!(kdoc_xref_name("@_x)"), Some("_x")); + // Escape sequences and names beginning with a digit are not references. + assert_eq!(kdoc_xref_name("\\n"), None); + assert_eq!(kdoc_xref_name("\\0"), None); + // Trailing punctuation must not smuggle an escape past the check: a + // clause ends with "\\n." as readily as it ends with "\\n". + assert_eq!(kdoc_xref_name("\\n."), None); + assert_eq!(kdoc_xref_name("\\t,"), None); + // The "@" spelling is unambiguous, so a param really named "n" works. + assert_eq!(kdoc_xref_name("@n"), Some("n")); + // A name cannot open with a digit, and "@" alone names nothing. + assert_eq!(kdoc_xref_name("@0"), None); + assert_eq!(kdoc_xref_name("@"), None); + assert_eq!(kdoc_xref_name("plain"), None); + // A path shape is not a reference even though it opens with a tag + // keyword: the dot leaves "file.txt" outside the identifier grammar. + assert_eq!(kdoc_xref_name("@file.txt"), None); + } + + #[test] + fn kernel_doc_param_cross_reference_is_description_text() { + // "@align" in prose names a param this comment declares, so it is a + // cross-reference, not a foreign tag: it rides along as description + // text instead of aborting the conversion. Trailing punctuation + // ("@align,") does not hide the reference. Shape from tlsf.h. + let lines = mk_lines(&[ + ("@param align Alignment in bytes", LineKind::Prose), + ( + "@param size Need not be a multiple of @align", + LineKind::Prose, + ), + ("@return Aligned to @align, or NULL", LineKind::Prose), + ]); + assert_eq!( + convert_bodies(lines), + vec![ + "@align : Alignment in bytes", + "@size : Need not be a multiple of @align", + "", + "Return Aligned to @align, or NULL", + ] + ); + + // A tag naming something the comment does NOT declare is still foreign, + // and still aborts. + let undeclared = mk_lines(&[ + ("@param x the x", LineKind::Prose), + ("@return bounded by @limit", LineKind::Prose), + ]); + assert_eq!( + convert_bodies(undeclared), + vec!["@param x the x", "@return bounded by @limit"] + ); + + // Head prose that merely mentions "@param note" declares nothing: the + // region opens at the first line whose FIRST word is a tag, and names + // are collected from there on. A real "@note" below is still foreign + // and still aborts, instead of being folded into "@x"'s description. + let prose_mention = mk_lines(&[ + ("Pass @param note to the logger.", LineKind::Prose), + ("@param x the x", LineKind::Prose), + ("@note beware", LineKind::Prose), + ]); + assert_eq!( + convert_bodies(prose_mention), + vec![ + "Pass @param note to the logger.", + "@param x the x", + "@note beware", + ] + ); + + // A param named after a real Doxygen tag does not turn that tag into a + // reference: the name is in DOXY_TAGS, so the whole comment aborts + // rather than folding "writer.c" into "@file"'s description. The name + // is the discriminator, not the position, because the position moves: + // reflow merges the aborted comment's lines and the next run would read + // the very same "@file" mid-line. + let collision = mk_lines(&[ + ("@param file the output stream", LineKind::Prose), + ("@file writer.c", LineKind::Prose), + ]); + assert_eq!( + convert_bodies(collision), + vec!["@param file the output stream", "@file writer.c"] + ); + + // Same name, mid-line: the name still decides, so this aborts too. + let midline = mk_lines(&[ + ("@param file the output stream", LineKind::Prose), + ("Opened before @file is read.", LineKind::Prose), + ]); + assert_eq!( + convert_bodies(midline), + vec![ + "@param file the output stream", + "Opened before @file is read." + ] + ); + + // The ceiling, pinned so it is a decision and not a surprise: a param + // named after a Doxygen command DOXY_TAGS does not list reads as a + // reference wherever it sits, so a real "@foo" section folds into the + // description. Position cannot rescue this (it is not stable under + // reflow); widening DOXY_TAGS can. + let custom_collision = mk_lines(&[ + ("@param foo the output stream", LineKind::Prose), + ("@foo custom section", LineKind::Prose), + ]); + assert_eq!( + convert_bodies(custom_collision), + vec!["@foo : the output stream @foo custom section"] + ); + + // The name scan reads the region as one word stream, so a "@param" + // ending a line still declares the name that opens the next one. Read + // line by line, "size" went undeclared, the comment aborted, and the + // run after (reflow having rejoined the two) converted it. + let wrapped_decl = mk_lines(&[ + ("@param", LineKind::Prose), + ("size the size in bytes", LineKind::Prose), + ("@return a multiple of @size", LineKind::Prose), + ]); + assert_eq!( + convert_bodies(wrapped_decl), + vec![ + "@size : the size in bytes", + "", + "Return a multiple of @size", + ] + ); + + // Trailing punctuation does not smuggle an undeclared name through. + let undeclared_punct = mk_lines(&[ + ("@param x the x", LineKind::Prose), + ("bounded by @limit, roughly", LineKind::Prose), + ]); + assert_eq!( + convert_bodies(undeclared_punct), + vec!["@param x the x", "bounded by @limit, roughly"] + ); + } + #[test] fn kernel_doc_bails_on_foreign_tag() { // An unconvertible tag (@note) leaves the whole comment untouched, diff --git a/src/reflow.rs b/src/reflow.rs index 061a38d..c7fdc3e 100644 --- a/src/reflow.rs +++ b/src/reflow.rs @@ -329,6 +329,31 @@ fn doxygen_hanging_indent(body: &str, effective: usize, flavor: DocFlavor) -> us return 0; } + // The kernel-doc form "convert_kernel_doc" emits ("@name : desc"). It is + // not a tag-table keyword, so the lookup below answers None and its + // continuations would wrap flush under the prefix while a "@param" entry's + // align under the description column. Both forms sit in one file the moment + // one comment converts and its neighbor aborts, so they have to agree. + // + // Safe only because the packer refuses to forge this shape mid-paragraph + // (see "would_forge_kdoc_tag" below). A nonzero hang is what makes an + // invented tag line visible: "classify_lines" has always split a paragraph + // at one, but with a flush wrap the regrouping moved no bytes. + // + // The description column is whatever follows the colon and its space. A + // name is "[A-Za-z0-9_]+", so everything up to the colon is ASCII and the + // byte offset is the column. + let trimmed = body.trim_start(); + if is_kernel_doc_tag(trimmed) { + let after_colon = trimmed.find(':').map_or(0, |i| i + 1); + let indent = after_colon + usize::from(trimmed[after_colon..].starts_with(' ')); + return if effective <= indent + MIN_WRAP_WIDTH { + CONTINUATION_INDENT + } else { + indent + }; + } + // Both spellings of a tag, "@param" and "\param", align the same way: strip // whichever marker is there and look the bare keyword up in the one tag // table. A keyword the table doesn't know, or one not followed by a space, @@ -388,6 +413,16 @@ fn is_tag_start(word: &str) -> bool { || is_kernel_doc_tag(word) } +/// A line holding nothing but "@name", the one shape a following ":"-led word +/// turns into the kernel-doc tag form. Every other way to end a line in "@name" +/// leaves an earlier word in front of it, and "is_kernel_doc_tag" reads the +/// line start, so those are already safe. +fn is_lone_kdoc_name(line: &str) -> bool { + line.strip_prefix('@').is_some_and(|name| { + !name.is_empty() && name.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') + }) +} + /// Greedy line-packing of one paragraph. The first line goes out behind /// "prefix" with the full "effective" width; every continuation line is /// indented a further "hang" columns and wraps that much earlier. @@ -446,13 +481,43 @@ fn wrap_segment_aligned( last_word = None; continue; } + // Appending is never guarded, only finalizing is. A line that packing // turned into a bookend ("-------- x --------") cannot escape: either a // later word does not fit, and the emit guard below refuses to send it // out, or the paragraph ends on it and the borrow at the bottom does. // Checking here as well was tried and deleted; it failed no test that // those two do not already cover, and it cost a scan per word. - if current_width + 1 + w_width <= avail { + // + // The one exception to "appending is never guarded", and it decides the + // append outright rather than voting on it: a lone "@name" about to + // take a ":"-led word is the kernel-doc tag shape, and which line it + // sits on is the whole question. Width does not get a say either way. + // + // On a CONTINUATION line the join FORGES a tag the source never had, + // and "classify_lines" splits a paragraph at one: the next pass + // regroups the text around it and the hanging indent moves with the + // regrouping. Falling through emits "@name" on its own and opens the + // next line with the ":" word, which loses nothing and breaks one word + // early. Guarding the finalize path instead cannot work here the way it + // does for bookends: "is_kernel_doc_tag" reads the line START, so once + // a line holds "@name :" no further word clears it and "keep packing" + // never terminates. The forging step is the only place to stand. + // + // On the paragraph's OPENING line the join is mandatory, because + // "@name" and its ":" are one unit: a first line stopping at "@name" is + // not a tag line on the next pass, so the paragraph merges into the one + // above and the hang goes with it. Overflow instead, the same trade the + // tag rule takes: an over-long line is recoverable, a regrouped one + // settles a run late. + let kdoc_colon_pending = + last_word.is_none() && w.starts_with(':') && is_lone_kdoc_name(¤t); + let takes_word = if kdoc_colon_pending { + on_first_line + } else { + current_width + 1 + w_width <= avail + }; + if takes_word { last_word = Some(current.len() + 1); current.push(' '); current.push_str(w); @@ -462,8 +527,8 @@ fn wrap_segment_aligned( // Past here the word does not fit, so a line is going out. - // Never hand the next pass a line the bookend stripper would rewrite. - // A rule token in prose ("a --- b") that a narrow wrap leaves alone on + // Never hand the next pass a line the bookend stripper would rewrite. A + // rule token in prose ("a --- b") that a narrow wrap leaves alone on // its line reads as a bare rule on the next run and is deleted, so the // file settles only on the second one. Same trade as the tag rule // below: keep packing and overflow, because an over-long line is @@ -486,10 +551,10 @@ fn wrap_segment_aligned( // That only works when the word moved down is not itself a tag; // otherwise the break just relocates the problem. With no such // word, let the line overflow. An over-long line is recoverable, a - // deleted word is not. - // Peeling must not undo the bookend guard above: breaking "--- x" - // in front of a tag emits "---" alone, which is the very line that - // guard exists to prevent. Fall through to the overflow arm. + // deleted word is not. Peeling must not undo the bookend guard + // above: breaking "--- x" in front of a tag emits "---" alone, + // which is the very line that guard exists to prevent. Fall through + // to the overflow arm. match last_word { Some(start) if !is_tag_start(¤t[start..]) @@ -520,12 +585,13 @@ fn wrap_segment_aligned( if current.is_empty() { return; } + // The same rule at the paragraph's end, where there is no next word to pack - // with. Borrow one from the line above so the rule is not alone, taking care - // that neither resulting line is a bookend either: folding the rule straight - // onto "-------- x y" would build the two-sided banner this is avoiding. - // Only the paragraph's first line has nothing to borrow from, and a - // paragraph that is one bare rule was the stripper's business long before + // with. Borrow one from the line above so the rule is not alone, taking + // care that neither resulting line is a bookend either: folding the rule + // straight onto "-------- x y" would build the two-sided banner this is + // avoiding. Only the paragraph's first line has nothing to borrow from, and + // a paragraph that is one bare rule was the stripper's business long before // reflow saw it. // // The tests below take the line above as a BODY, not as the emitted line. @@ -544,16 +610,30 @@ fn wrap_segment_aligned( .strip_prefix(prev_prefix) .unwrap_or(&out[last]) .to_string(); - if let Some(space) = prev_body.rfind(' ') { - let kept = &prev_body[..space]; - let joined = format!("{} {current}", &prev_body[space + 1..]); - if bookend_match(kept).is_none() && bookend_match(&joined).is_none() { - out[last] = format!("{prev_prefix}{kept}"); - current = joined; - } else if bookend_match(&format!("{prev_body} {current}")).is_none() { - out[last] = format!("{prev_prefix}{prev_body} {current}"); - return; - } + + // The borrowed word opens the paragraph's last line, and that line is + // always a continuation, so the tag rule above applies to it just as it + // does to a mid-paragraph break: a tag parked at the first column is + // read by "classify_lines" as its own tag paragraph, the next pass + // regroups around it, and the hanging indent moves with the regrouping. + // + // When there is nothing to borrow, or borrowing would build one of the + // shapes above, fold the rule onto the line above whole. That arm used + // to sit inside the "there is a word to borrow" case, so a line holding + // ONE word (no space to split at) fell through and emitted the rule + // alone anyway, which the next pass reads as a bare rule and deletes. + let borrow = prev_body.rfind(' ').filter(|&space| { + let borrowed = &prev_body[space + 1..]; + !is_tag_start(borrowed) + && bookend_match(&prev_body[..space]).is_none() + && bookend_match(&format!("{borrowed} {current}")).is_none() + }); + if let Some(space) = borrow { + out[last] = format!("{prev_prefix}{}", &prev_body[..space]); + current = format!("{} {current}", &prev_body[space + 1..]); + } else if bookend_match(&format!("{prev_body} {current}")).is_none() { + out[last] = format!("{prev_prefix}{prev_body} {current}"); + return; } } let line_prefix = if out.len() == first_line_mark { diff --git a/src/signature.rs b/src/signature.rs index f7fbd74..657d63a 100644 --- a/src/signature.rs +++ b/src/signature.rs @@ -85,8 +85,9 @@ fn detect_param_drift( trailing.push(child); } } - // Exactly one, which is what the tell means: a single comment displaced - // off the end of the list. Two or more after ")" is not one-step drift, and + + // Exactly one, which is what the tell means: a single comment displaced off + // the end of the list. Two or more after ")" is not one-step drift, and // shifting them all onto the last parameter invents a grouping the author // never wrote. The doc comment above has always said "exactly one"; the // code only checked "at least one". diff --git a/tests/cli.rs b/tests/cli.rs index f7f333c..ed1a710 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -32,11 +32,12 @@ fn run(args: &[&str], input: &[u8]) -> (Option, String, String) { .stderr(Stdio::piped()) .spawn() .expect("spawn"); - // A run that rejects its arguments exits before it ever reads stdin, so this - // write races that exit and loses whenever the child wins: EPIPE. That is - // the binary behaving correctly, and the assertions that follow are about - // the exit code and stderr, which wait_with_output still collects. Only a - // real I/O failure is worth failing a test over. + + // A run that rejects its arguments exits before it ever reads stdin, so + // this write races that exit and loses whenever the child wins: EPIPE. That + // is the binary behaving correctly, and the assertions that follow are + // about the exit code and stderr, which wait_with_output still collects. + // Only a real I/O failure is worth failing a test over. let mut stdin = child.stdin.take().unwrap(); if let Err(e) = stdin.write_all(input) && e.kind() != std::io::ErrorKind::BrokenPipe diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 61a98d9..92a2a0f 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -31,7 +31,10 @@ pub fn pipeline(source: &str, lang: Language, column_limit: usize) -> String { // misses. let reps2 = commentflow::plan(&out, lang, column_limit, indent_cfg, &mut pool).unwrap(); let out2 = rewrite::apply(&out, &reps2); - assert_eq!(out, out2, "second pass changed the output (not idempotent)"); + assert_eq!( + out, out2, + "second pass changed the output (not idempotent)\n--- source\n{source}--- pass 1\n{out}--- pass 2\n{out2}" + ); out } diff --git a/tests/convergence.rs b/tests/convergence.rs index 20fb6e6..567fa57 100644 --- a/tests/convergence.rs +++ b/tests/convergence.rs @@ -86,6 +86,24 @@ const VOCAB: &[&str] = &[ "@len:", "@epsilon", ":", + // Bare words that double as Doxygen tag keywords. A param declared with one + // of these ("@param note") makes a later "@note" spell exactly like the + // reference "is_param_xref" exempts, so its "DOXY_TAGS" membership test is + // the only thing keeping a real section out of the preceding param's + // description. Without these in the vocabulary the collision is + // ungeneratable. + // + // Both need a BARE tag token above to collide with, which is why "file" is + // not here despite being the most realistic collision of the three: the + // only file-shaped token is the path "@file.txt", and "kdoc_xref_name" + // rejects it on the dot, so it aborts as a foreign tag whether or not a + // param shares the name. Adding a bare "@file" makes the collision + // generatable and immediately trips a pre-existing blank-line bug unrelated + // to the exemption, so the file case is pinned by hand instead, in + // "kernel_doc_param_cross_reference_is_description_text" and + // "doxygen_section_tag_survives_a_param_of_the_same_name". + "note", + "warning", // Tag names outside "[A-Za-z]", which "is_foreign_tag" does not claim. "@1buf", "@1buf:", diff --git a/tests/invariants.rs b/tests/invariants.rs index 8283d79..a7b8337 100644 --- a/tests/invariants.rs +++ b/tests/invariants.rs @@ -320,8 +320,8 @@ fn return_on_own_line_prose_capital() { #[test] fn return_on_own_line_doxygen_at_return() { // C/C++ Doxygen return tags convert to kernel-doc "Return ..." on their own - // line, blank-separated. All four spellings (@return, @returns, \return, - // \returns) converge to the same output. + // line, blank-separated. All four spellings (@return, @returns, \return, \returns) + // converge to the same output. let src = "#include \n/**\n * Computes a thing.\n * @return the result of the computation\n */\nint f(void) { return 0; }\n"; let out = pipeline(src, detect("foo.c"), 80); assert_kernel_doc_return(&out); @@ -1192,9 +1192,9 @@ fn rustdoc_doc_flavor_cpp_uses_doxygen() { #[test] fn rustdoc_doc_flavor_rs_treats_at_param_as_prose() { - // Same "///" + "@param ..." text in a .rs file. Under Rustdoc flavor, - // @param is just prose: continuation is flush-left under "/// ", NOT - // description-column aligned. + // Same "///" + "@param ..." text in a .rs file. Under Rustdoc + // flavor, @param is just prose: continuation is flush-left under "/// ", + // NOT description-column aligned. let src = "/// Computes a thing.\n/// @param widget the widget object that needs careful operation here in this rust file using prose semantics\nfn f(widget: i32) -> i32 { widget }\n"; let out = pipeline(src, detect("foo.rs"), 80); let cont = out diff --git a/tests/pipeline.rs b/tests/pipeline.rs index 4214c36..6a259c0 100644 --- a/tests/pipeline.rs +++ b/tests/pipeline.rs @@ -803,6 +803,209 @@ fn doxygen_param_converts_to_kernel_doc() { assert_eq!(out, pass2, "kernel-doc conversion must be idempotent"); } +#[test] +fn doxygen_param_cross_reference_survives_conversion() { + // A comment that cross-references its own arguments in prose ("a multiple + // of @align") used to abort the whole conversion, since any "@word" that is + // not @param/@return read as a foreign tag. Shape from tlsf-bsd's tlsf.h. + // The narrow limit forces the references across wrap boundaries, where + // "common::pipeline"'s second-pass assertion is the real test. + let src = "/**\n * Allocate memory with a specified alignment.\n *\n * @param t the allocator\n * @param align alignment in bytes, a power of two\n * @param size bytes wanted; need not be a multiple of @align\n * @return pointer aligned to @align, or NULL on failure\n */\nvoid *f(void *t, size_t align, size_t size);\n"; + let out = pipeline(src, detect("foo.c"), 56); + + assert!( + out.contains(" * @align : alignment in bytes"), + "params must convert, got:\n{out}" + ); + assert!( + !out.contains("@param") && !out.contains("@return"), + "no Doxygen param/return tag may survive, got:\n{out}" + ); + + // Both prose references survive as words. Where reflow parks them is not + // pinned here: a bare "@align" is not a tag any pass eats, and making the + // packer keep it off a continuation-line start cost a fixed point. + assert_eq!( + out.matches("@align").count(), + 3, + "one declaration plus both references must survive, got:\n{out}" + ); + assert!( + out.contains("aligned to"), + "the return description must survive, got:\n{out}" + ); +} + +#[test] +fn doxygen_backslash_param_cross_reference_survives_conversion() { + let src = "/**\n * \\param size bytes wanted\n * \\return pointer to \\size, or NULL on failure\n */\nvoid *f(size_t size);\n"; + let out = pipeline(src, detect("foo.c"), 80); + + assert!( + out.contains(" * @size : bytes wanted") + && out.contains("Return pointer to \\size, or NULL on failure"), + "backslash parameter references must convert, got:\n{out}" + ); + assert!(!out.contains("\\param") && !out.contains("\\return")); +} + +#[test] +fn doxygen_foreign_tag_still_aborts_despite_a_prose_param_mention() { + // Head prose mentioning "@param note" is not a declaration, so the real + // "@note" section below stays foreign and the whole comment passes through + // rather than folding "@note" into "@x"'s description. + let src = "/**\n * Pass @param note to the logger when tracing.\n *\n * @param x the x value\n * @note beware of the reentrancy hazard\n */\nint f(int x);\n"; + let out = pipeline(src, detect("foo.c"), 80); + assert_eq!(out, src, "a foreign @note must leave the comment untouched"); +} + +#[test] +fn doxygen_section_tag_survives_a_param_of_the_same_name() { + // "file" is an ordinary C parameter name and also a Doxygen section tag. + // The name is what disqualifies it as a cross-reference, so the section + // survives and the whole comment passes through rather than folding + // "writer.c" into the param. + let src = "/**\n * @param file the output stream\n * @file writer.c\n * @return count\n */\nint f(void *file);\n"; + let out = pipeline(src, detect("foo.c"), 80); + assert_eq!( + out, src, + "a real @file section must leave the comment alone" + ); +} + +#[test] +fn doxygen_section_tag_survives_a_reflow_that_merges_its_line() { + // The same collision in a comment with no doc flavor, where nothing marks + // "@file" as a tag line and reflow packs the whole body onto one line. A + // position-based reference test read the merged "@file" as running prose + // and converted on the SECOND run what the first refused, folding the + // section into the param description. "common::pipeline" asserts the fixed + // point, so the merge here is the whole test. + let src = "/*\n * @param file the output stream\n * @file writer.c\n */\nint f(void *file);\n"; + let out = pipeline(src, detect("foo.c"), 80); + assert!( + out.contains("@param file") && out.contains("@file writer.c"), + "a merged @file section must still abort the conversion, got:\n{out}" + ); +} + +#[test] +fn doxygen_param_declares_a_name_that_wrapped_off_its_tag_line() { + // "@param" ending a line takes its name off the line below, exactly as the + // entry scan reads it. Collected per line the name went undeclared, so the + // "@size" reference read as a foreign tag and aborted; reflow then rejoined + // the tag and its name, and the next run converted. One run has to do it. + let src = "/**\n * @param\n * size the size in bytes\n * @return a multiple of @size\n */\nvoid *f(size_t size);\n"; + let out = pipeline(src, detect("foo.c"), 80); + assert!( + out.contains(" * @size : the size in bytes"), + "the wrapped param must convert on the first run, got:\n{out}" + ); + assert!( + out.contains("Return a multiple of @size"), + "the reference must ride along as description text, got:\n{out}" + ); +} + +#[test] +fn doxygen_param_named_after_a_convertible_tag_passes_through() { + // "@param return desc" would emit "@return : desc", which the next run + // reads as a return tag and rewrites again to "Return : desc". A param name + // that is itself a convertible keyword cannot round-trip, so the comment + // passes through instead. + let src = "/**\n * @param return the return slot\n */\nint f(int *ret);\n"; + let out = pipeline(src, detect("foo.c"), 80); + assert_eq!(out, src, "a param named \"return\" must not convert"); +} + +#[test] +fn kernel_doc_entry_hangs_its_continuations_under_the_description() { + // The form "convert_kernel_doc" emits wraps under the description column, + // the same way a "@param" entry does. Shape from tlsf-bsd's tlsf.h. + let src = "/**\n * @prev : Pointer to the previous physical block. Only valid when the previous block is free; physically stored at the tail of that block's payload.\n * @header : Size or status bits.\n */\nstruct b { int prev; };\n"; + let out = pipeline(src, detect("foo.c"), 80); + // Assert the column rather than a hardcoded run of spaces: the + // continuation must begin exactly under "Pointer". + let lines: Vec<&str> = out.lines().collect(); + let head = lines.iter().position(|l| l.contains("@prev :")).unwrap(); + let desc_col = lines[head].find("Pointer").unwrap(); + let cont_col = lines[head + 1].find("block is free;").unwrap(); + assert_eq!( + cont_col, desc_col, + "continuation must align under the description column, got:\n{out}" + ); + // A short entry that never wraps keeps its single line. + assert!( + out.contains("\n * @header : Size or status bits.\n"), + "an entry that fits must not gain an indent, got:\n{out}" + ); +} + +#[test] +fn packer_never_forges_a_kernel_doc_tag_mid_paragraph() { + // A lone "@buf" that takes a ":"-led word on a continuation line would be + // a tag line the source never had. "classify_lines" splits a paragraph at + // one, so the next pass regroups the text and the hanging indent moves + // with it. The harness's second-pass assertion is the real test here. + let src = "/**\n * alpha beta gamma delta epsilon zeta eta theta iota kappa @buf : lambda mu\n */\nint f(int buf);\n"; + let out = pipeline(src, detect("foo.c"), 46); + assert!( + !out.lines().any(|l| l.trim_start().starts_with("* @buf :")), + "reflow must not open a continuation line with a forged tag, got:\n{out}" + ); +} + +#[test] +fn packer_never_splits_a_kernel_doc_entry_from_its_colon() { + // The mirror: a name long enough to fill the opening line must still keep + // its ":" , overflowing if it has to. A first line ending at "@name" is + // not a tag line on the next pass, so the entry would dissolve. + let src = "/**\n * lead words here\n * @destination_buffer_length : alpha beta gamma delta epsilon\n */\nint f(int destination_buffer_length);\n"; + let out = pipeline(src, detect("foo.c"), 34); + assert!( + out.contains("* @destination_buffer_length :"), + "the name must keep its colon, got:\n{out}" + ); +} + +#[test] +fn packer_never_borrows_a_tag_onto_a_continuation_line() { + // A paragraph ending in a bare rule borrows a word from the line above so + // the rule is not stranded there, and that borrowed word OPENS the last + // line, which is a continuation. Borrowing a tag onto it is the same defect + // the tag rule prevents at every other break: "classify_lines" reads a tag + // at the first column as its own tag paragraph, the next pass regroups + // around it, and the hanging indent moves with the regrouping. Invisible + // while a kernel-doc entry wrapped flush; the moment it hangs, the regroup + // moves bytes and the file settles only on its second run. + // + // The trailing "---" is an em-dash, the shape the bookend rules exist for. + // "common::pipeline" asserts the fixed point, so the borrow is the test. + let src = + "/**\n * @buf : destination for the decoded bytes, see @note ---\n */\nint f(int x);\n"; + let out = pipeline(src, detect("foo.c"), 24); + assert!( + !out.lines().any(|l| l.trim_start().starts_with("* @note")), + "the borrow must not open a continuation line with a tag, got:\n{out}" + ); +} + +#[test] +fn a_rule_never_goes_out_alone_behind_a_one_word_line() { + // The other way out of the borrow: the line above holds a single word, so + // there is no space to split it at and nothing to lend. The fold arm has to + // catch that, because falling through emits the rule alone, and the next + // pass reads a line that is nothing but a rule run as a bare decorative + // rule and DELETES it. A deleted word is the one outcome worth overflowing + // a line to avoid. + let src = "/**\n * @param[in] warning multiple beta reallyquitelongword ---\n */\nvoid *f(void *t);\n"; + let out = pipeline(src, detect("foo.c"), 25); + assert!( + out.contains("reallyquitelongword ---"), + "the rule must ride out on the line above, got:\n{out}" + ); +} + #[test] fn mid_paragraph_doxygen_param_does_not_lose_tag_in_line_run() { let src =