diff --git a/UPGRADING.md b/UPGRADING.md index f983c8cc..02f7cd8d 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,5 +1,57 @@ # Upgrading Markbridge +## Unreleased — AST normalization runs by default + +A new `Markbridge::Normalizer` pass runs between the parse-time `yield` hook +and rendering. It rewrites the AST so the renderer only gets markup the target +format can express. It is **on by default** for every `*_to_markdown` call, +`convert`, and `render`. + +What changes in the output, with no code change on your side: + +- A link inside a link (`[url][url]…[/url][/url]`) collapses to a single link. + CommonMark does not allow nested links. +- A block element inside an inline container — a quote, list, table, or a + `Poll`/`Event` node inside a link, bold, or a heading — is moved out, so the + inline element does not break. This is not link-specific. +- A fenced or multi-line code block inside an inline container is moved out; a + one-line code span stays. +- A formatting wrapper left empty by the above is removed (no empty `**` `**`). + An empty link is kept, because it renders as a plain URL. + +Each change is reported under `conversion.diagnostics[:normalization]`, next +to `unknown_tags`. + +The default rules are legality only. Discourse policy is not built in. A +linked image (`[![alt](src)](url)`) is valid CommonMark, so the default leaves +it alone. To move image-likes out of links, add the rules yourself. This is +the pattern that replaces hoisting logic a consumer used to have in a custom +`Url` tag: + +```ruby +NORMALIZER = + Markbridge::Normalizer + .default + .rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :hoist_after) + .rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Upload, strategy: :hoist_after) + .rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Attachment, strategy: :hoist_after) + .freeze + +Markbridge.convert(input, format: :bbcode, normalize: NORMALIZER) +``` + +Build the normalizer once (a constant) and reuse it. `#normalize` keeps no +state on the instance, so one frozen instance is safe for every conversion, +also across threads, and passing it is as fast as the default path. + +To skip normalization: + +```ruby +Markbridge.convert(input, format: :bbcode, normalize: false) +``` + +See [docs/normalization.md](docs/normalization.md). + ## 0.3.0 — quote attribution fields and URL rendering ### `AST::Quote` attribution fields renamed and typed diff --git a/bench/corpus_bench.rb b/bench/corpus_bench.rb index b1b440db..ebec8ccf 100644 --- a/bench/corpus_bench.rb +++ b/bench/corpus_bench.rb @@ -43,9 +43,22 @@ def report(name, corpus, &work) ) end +# A violation-heavy variant of a corpus: prepends a few degenerate +# constructs (linked image, nested links, block-in-link) to each post so +# the normalizer's worst case — including the destination-stack rewalk on +# moved subtrees — is exercised on realistic surrounding text. +def build_heavy(corpus) + violating = + "[url=https://ex.com/a][img]https://ex.com/i.png[/img][/url] " \ + "[url=https://a.com][url=https://b.com]x[/url][/url] " \ + "[url=https://ex.com][quote]q[/quote][/url] " + corpus.map { |post| violating + post } +end + # ASCII/multibyte corpus pair per source format. CORPORA = { bbcode: -> { [Corpus.ascii, Corpus.multibyte] }, + bbcode_heavy: -> { [build_heavy(Corpus.ascii), build_heavy(Corpus.multibyte)] }, mediawiki: -> { [Corpus.mediawiki, Corpus.mediawiki_multibyte] }, html: -> { [Corpus.html, Corpus.html_multibyte] }, text_formatter: -> { [Corpus.text_formatter, Corpus.text_formatter_multibyte] }, @@ -98,6 +111,44 @@ def report(name, corpus, &work) end end, ], + # The default-on gate: the normalization walk over violation-free trees + # in isolation (pre-parsed ASTs; the clean corpus never mutates, so every + # round measures the same zero-violation traversal). Compare against zero. + "norm_only" => [ + :bbcode, + lambda do |corpus, tag| + parser = Markbridge::Parsers::BBCode::Parser.new + asts = corpus.map { |post| parser.parse(post) } + normalizer = Markbridge::Normalizer.shared_for(:discourse) + report("norm_only/#{tag}", asts) { |ast| normalizer.normalize(ast) } + end, + ], + # End-to-end baseline with normalization off; `fresh` minus this is the + # zero-violation overhead of default-on normalization. + "fresh_no_norm" => [ + :bbcode, + lambda do |corpus, tag| + report("fresh_no_norm/#{tag}", corpus) do |post| + Markbridge.bbcode_to_markdown(post, normalize: false) + end + end, + ], + # Worst-case bound: end-to-end over a violation-heavy corpus, with and + # without normalization (the difference is the mutation + rewalk cost). + "norm_heavy" => [ + :bbcode_heavy, + lambda do |corpus, tag| + report("norm_heavy/#{tag}", corpus) { |post| Markbridge.bbcode_to_markdown(post) } + end, + ], + "norm_heavy_off" => [ + :bbcode_heavy, + lambda do |corpus, tag| + report("norm_heavy_off/#{tag}", corpus) do |post| + Markbridge.bbcode_to_markdown(post, normalize: false) + end + end, + ], "mw_fresh" => [ :mediawiki, lambda do |corpus, tag| diff --git a/docs/architecture.md b/docs/architecture.md index 5922c27b..f1c60300 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -26,6 +26,12 @@ Markbridge follows a **Parse → AST → Render** architecture: Registry Library ``` +Between the AST and Render, an optional `Markbridge::Normalizer` pass +rewrites the tree so the renderer is only handed markup the target format +can express (no link inside a link, no block inside an inline container, +etc.). It runs by default at the conversion level; the three components +above are unchanged by it. See [AST Normalization](normalization.md). + ### Phase 1: Parsing (BBCode → AST) **Purpose:** Convert BBCode text into a structured tree diff --git a/docs/normalization.md b/docs/normalization.md new file mode 100644 index 00000000..a92cf5e5 --- /dev/null +++ b/docs/normalization.md @@ -0,0 +1,123 @@ +# AST Normalization + +Markup can nest elements in ways Markdown cannot express: a link inside a +link, a block element inside an inline container (a link label, but also bold +or a heading), a fenced code block inside emphasis. If the renderer prints +these as-is, the Markdown breaks — the inner link wins and the outer one turns +into text, a block's blank lines break out of the emphasis around it. + +`Markbridge::Normalizer` walks the AST once, between the parse-time `yield` +hook and rendering, and rewrites it so the renderer only gets markup the +target format can express. It runs **by default**. The renderer's tags stay +simple string emitters; the rules about what may nest in what live here +instead. + +## Where it runs + +``` +parse → yield(ast) → normalize → render +``` + +Because it runs after the `yield` hook, changes you make to the AST in that +block are normalized too. It runs for every source format and for +`Markbridge.render`, because normalization is about the *target* format, not +the source. + +## The default rules + +The default rules are CommonMark legality. Break one and the Markdown does +not parse back as the tree meant: + +- No link inside a link, at any depth (§6.3). The inner link is unwrapped. +- An inline container holds inline content only, so a block element inside one + is moved out. This is not link-specific: emphasis (`Bold`, `Italic`, …) and + headings are inline containers too, so a poll inside bold or a list inside a + heading is handled the same way as a block inside a link. The lists are + `Normalizer::INLINE_CONTAINERS` and `Normalizer::BLOCK_NODES` (which covers + `List`, `Table`, `Quote`, `Details`, `HorizontalRule`, `Align`, and the + Discourse `Poll`/`Event` nodes). +- A code span inside an inline container is fine while it stays on one line. A + fenced or multi-line block is moved out. + +Discourse-specific policy is **not** built in. Moving an image out of a link, +for example, is a rule you add yourself (see below). A linked image +(`[![alt](src)](url)`) is valid CommonMark, so the default leaves it alone. + +## Strategies + +Each match resolves to one strategy: + +| Strategy | Effect | +|----------|--------| +| `:keep` | Allow it. This records a decision and keeps it out of the report. | +| `:hoist_after` | Move the node out and put it right after the outermost matching ancestor, keeping the document order. An image in a bold that sits in a link is moved after the whole link (out of both), because the bold is inside the link. The walker only moves a node out to a sibling; it never puts one into a wrapper it was not already in. | +| `:unwrap` | Remove the element and put its children in its place. The built-in case is a link inside a link: `[[text](inner)](outer)` becomes `[text](outer)`. The inner link and its href are dropped; its text stays under the outer link. | +| `:textify` | Replace the subtree with its plain text (`@name` for a mention, the joined text otherwise). | +| `:drop` | Remove it. | +| callable | `->(boundary, node) { … }` that returns a strategy symbol, an `Array` to put in its place, or `nil` to drop it. Use this for anything the built-in strategies do not cover. | + +A formatting wrapper (bold, italic, color, …) that ends up empty after a +hoist or drop is removed, so no empty `**` `**` markers are left. A link is +the exception: an empty link is kept, because it renders as a plain URL. + +## Diagnostics + +Every change is reported through the same channel as `unknown_tags`: + +```ruby +conversion = Markbridge.convert(input, format: :bbcode) +conversion.diagnostics[:normalization] +# => [{ parent: "Url", child: "Url", strategy: :unwrap, count: 1 }] +``` + +For a migration this feeds per-post warnings and shows which sources produce +broken trees. The key is absent when nothing changed. + +## Opting out and customizing + +`normalize:` takes `true` (default, the shared normalizer), `false` (skip), or +a `Normalizer` instance: + +```ruby +# Skip normalization +Markbridge.convert(input, format: :bbcode, normalize: false) + +# Add your own rules on top of the defaults +normalizer = Markbridge::Normalizer.default +normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :hoist_after) +Markbridge.convert(input, format: :bbcode, normalize: normalizer) +``` + +Build a customized normalizer once and reuse it. `#normalize` and +`#violations` keep no state on the instance, so one instance (freeze it if you +like) is safe to use for every conversion, also across threads. Passing your +own instance is as fast as the default path — there is no per-call rule +build. + +A rule for a `(parent, child)` pair that already exists is replaced, so your +`#rule` calls override the defaults. Matching is by exact class +(`instance_of?`), so a rule for `AST::Url` does not catch a subclass. + +`Markbridge::Normalizer.shared_default` is the default normalizer, built once +and frozen; the `normalize: true` path uses it. Do not change it — call +`.default` for a fresh, customizable one. + +## Validation + +The same rules, without changing the tree: + +```ruby +Markbridge::Normalizer.default.violations(ast) +# => [{ parent: "Url", child: "Url", strategy: :unwrap }] +``` + +Two uses: check in your own test suite that the trees your parsers and tag +fixtures build have no violations, or run it as a lint over a corpus without +changing any output. After a `normalize`, `violations` returns nothing — +normalization is done in a single pass. + +## Adding a target format + +`Normalizer.default` builds the rule table; the engine (`RuleSet`, `Walker`) +does not care about the format. A second target would add another builder and +a matching class method next to `default`; nothing else changes. diff --git a/docs/renderers/discourse.md b/docs/renderers/discourse.md index fecc903c..9b2a5b7a 100644 --- a/docs/renderers/discourse.md +++ b/docs/renderers/discourse.md @@ -958,8 +958,17 @@ doc = AST::Document.new([ renderer.render(doc) # => "content" (Unknown wrapper ignored) ``` +## AST normalization + +The renderer's tags are simple string emitters. They assume the tree is +already legal for the target format (no link inside a link, no block element +inside an inline container like bold or a heading). Making it legal is the job +of a separate pass, `Markbridge::Normalizer`, which runs by default between +parse and render. See **[AST Normalization](../normalization.md)**. + ## Next Steps +- **[AST Normalization](../normalization.md)** - Target-format nesting rules applied before rendering - **[BBCode Parser Guide](../parsers/bbcode.md)** - Learn how to build AST from BBCode - **[Extending Markbridge](../extending.md)** - Add custom tags and renderers - **[Architecture Overview](../architecture.md)** - Understand the full pipeline diff --git a/lib/markbridge.rb b/lib/markbridge.rb index df1753c9..6b565088 100644 --- a/lib/markbridge.rb +++ b/lib/markbridge.rb @@ -5,6 +5,7 @@ require_relative "markbridge/conversion" require_relative "markbridge/ast" +require_relative "markbridge/normalizer" require_relative "markbridge/renderers/discourse" module Markbridge @@ -44,11 +45,21 @@ def parse_bbcode(input, handlers: nil) # {Conversion} with an empty +markdown+ string, and surface the # exceptions via {Conversion#errors}. # @yieldparam ast [AST::Document] mutate before rendering (optional) + # @param normalize [Boolean, Normalizer] apply target-format nesting + # rules between the +yield+ hook and render. +true+ (default) uses the + # shared default normalizer; a {Normalizer} is used as-is; +false+ + # skips normalization. See {Normalizer}. # @return [Conversion] - def bbcode_to_markdown(input, handlers: nil, renderer: nil, raise_on_error: true) + def bbcode_to_markdown( + input, + handlers: nil, + renderer: nil, + raise_on_error: true, + normalize: true + ) parse = parse_bbcode(input, handlers:) yield(parse.ast) if block_given? - build_conversion(parse, renderer:, raise_on_error:) + build_conversion(parse, renderer:, raise_on_error:, normalize:) end # Parse HTML to AST. @@ -85,11 +96,12 @@ def parse_html(input, handlers: nil) # {Conversion} with an empty +markdown+ string, and surface the # exceptions via {Conversion#errors}. # @yieldparam ast [AST::Document] mutate before rendering (optional) + # @param normalize [Boolean, Normalizer] see {.bbcode_to_markdown} # @return [Conversion] - def html_to_markdown(input, handlers: nil, renderer: nil, raise_on_error: true) + def html_to_markdown(input, handlers: nil, renderer: nil, raise_on_error: true, normalize: true) parse = parse_html(input, handlers:) yield(parse.ast) if block_given? - build_conversion(parse, renderer:, raise_on_error:) + build_conversion(parse, renderer:, raise_on_error:, normalize:) end # Parse s9e/TextFormatter XML to AST. @@ -122,11 +134,18 @@ def parse_text_formatter_xml(input, handlers: nil) # @param renderer [Renderers::Discourse::Renderer, nil] custom renderer # @param raise_on_error [Boolean] see {.bbcode_to_markdown} # @yieldparam ast [AST::Document] mutate before rendering (optional) + # @param normalize [Boolean, Normalizer] see {.bbcode_to_markdown} # @return [Conversion] - def text_formatter_xml_to_markdown(input, handlers: nil, renderer: nil, raise_on_error: true) + def text_formatter_xml_to_markdown( + input, + handlers: nil, + renderer: nil, + raise_on_error: true, + normalize: true + ) parse = parse_text_formatter_xml(input, handlers:) yield(parse.ast) if block_given? - build_conversion(parse, renderer:, raise_on_error:) + build_conversion(parse, renderer:, raise_on_error:, normalize:) end # Parse MediaWiki wikitext to AST. @@ -155,11 +174,18 @@ def parse_mediawiki(input, handlers: nil) # @param renderer [Renderers::Discourse::Renderer, nil] custom renderer # @param raise_on_error [Boolean] see {.bbcode_to_markdown} # @yieldparam ast [AST::Document] mutate before rendering (optional) + # @param normalize [Boolean, Normalizer] see {.bbcode_to_markdown} # @return [Conversion] - def mediawiki_to_markdown(input, handlers: nil, renderer: nil, raise_on_error: true) + def mediawiki_to_markdown( + input, + handlers: nil, + renderer: nil, + raise_on_error: true, + normalize: true + ) parse = parse_mediawiki(input, handlers:) yield(parse.ast) if block_given? - build_conversion(parse, renderer:, raise_on_error:) + build_conversion(parse, renderer:, raise_on_error:, normalize:) end # Convert input in the given format. Thin dispatcher over the @@ -211,8 +237,17 @@ def convert(input, format:, **kwargs, &block) # @param format [Symbol] :discourse (only renderer currently shipped) # @param renderer [Renderers::Discourse::Renderer, nil] # @param raise_on_error [Boolean] + # @param normalize [Boolean, Normalizer] see {.bbcode_to_markdown}. + # Normalization is idempotent, so re-rendering an already-normalized + # {Parse} is a no-op. # @return [Conversion] - def render(parse_or_ast, format: :discourse, renderer: nil, raise_on_error: true) + def render( + parse_or_ast, + format: :discourse, + renderer: nil, + raise_on_error: true, + normalize: true + ) raise ArgumentError, "unknown render format #{format.inspect}" unless format == :discourse parse = @@ -228,7 +263,7 @@ def render(parse_or_ast, format: :discourse, renderer: nil, raise_on_error: true raise ArgumentError, "expected Parse or AST::Node, got #{parse_or_ast.class}" end - build_conversion(parse, renderer:, raise_on_error:) + build_conversion(parse, renderer:, raise_on_error:, normalize:) end # Build a configured Discourse {Renderers::Discourse::Renderer} @@ -297,13 +332,28 @@ def bbcode_diagnostics(parser) } end - def build_conversion(parse, renderer:, raise_on_error:) + def build_conversion(parse, renderer:, raise_on_error:, normalize:) + parse = apply_normalization(parse, normalize) renderer ||= Renderers::Discourse::Renderer.new markdown, errors = render_through(renderer, parse.ast, raise_on_error:) Conversion.new(parsed: parse, markdown:, errors:) end + # Normalize +parse.ast+ in place (target-format nesting rules) and fold + # any report into +diagnostics[:normalization]+. Returns the +parse+ to + # render — a new one carrying the report when something changed, else + # the original unchanged. + def apply_normalization(parse, normalize) + return parse unless normalize + + normalizer = normalize.is_a?(Normalizer) ? normalize : Normalizer.shared_default + report = normalizer.normalize(parse.ast) + return parse if report.empty? + + parse.with(diagnostics: parse.diagnostics.merge(normalization: report)) + end + def build_escaper(escape:, escape_hard_line_breaks:, allow:) if escape == false if escape_hard_line_breaks || allow diff --git a/lib/markbridge/ast/element.rb b/lib/markbridge/ast/element.rb index 99fe7934..73479a84 100644 --- a/lib/markbridge/ast/element.rb +++ b/lib/markbridge/ast/element.rb @@ -105,6 +105,34 @@ def replace_child(old_child, new_child) @children[index] = new_child self end + + # Replace this element's entire child list in one shot. + # + # A plain validated setter for tree-rewriting passes (e.g. the + # {Markbridge::Normalizer}) that rebuild an element's children out + # of band and need to commit the result without re-running the + # per-append logic of {#<<}. Every entry must be a {Node}. + # + # NOTE: unlike {#<<}, this does *not* merge adjacent {Text} nodes — + # the auto-merge invariant is a property of {#<<} only. Callers that + # build the array themselves own that coalescing (the Normalizer's + # walker merges adjacent text as it assembles the list, so it never + # hands a state {#<<} would not have produced). + # + # @param new_children [Array] the replacement children + # @return [Element] +self+ + # @raise [TypeError] when any entry is not a {Node} + def replace_children(new_children) + new_children.each do |child| + next if child.is_a?(Node) + + actual = child.nil? ? "nil" : child.class + raise TypeError, "replace_children on #{self.class} expected #{Node}s, got #{actual}" + end + + @children = new_children + self + end end end end diff --git a/lib/markbridge/normalizer.rb b/lib/markbridge/normalizer.rb new file mode 100644 index 00000000..3b3e66b1 --- /dev/null +++ b/lib/markbridge/normalizer.rb @@ -0,0 +1,180 @@ +# frozen_string_literal: true + +require_relative "normalizer/rule_set" +require_relative "normalizer/report" +require_relative "normalizer/text_projection" +require_relative "normalizer/walker" + +module Markbridge + # Rewrites an AST so the renderer only gets markup the target format can + # express. It runs once, between parse and render. The default rules are + # CommonMark legality: no link inside a link, no block element inside an + # inline container, and an inline-only code span. Each match resolves to a + # strategy (+:keep+, +:hoist_after+, +:unwrap+, +:textify+, +:drop+, or a + # callable) that the {Walker} applies. + # + # Discourse-specific policy (for example, moving an image out of a link) is + # not built in. A consumer adds those with {#rule}. + # + # @example The default, reused across conversions + # Markbridge::Normalizer.shared_default + # + # @example A customized normalizer + # n = Markbridge::Normalizer.default + # n.rule(parent: AST::Url, child: AST::Image, strategy: :hoist_after) + # Markbridge.convert(input, format: :bbcode, normalize: n) + # + # @example List what would change, without changing it + # Markbridge::Normalizer.default.violations(ast) # => [...] + class Normalizer + # The strategy symbols a rule may resolve to. + STRATEGIES = %i[keep hoist_after unwrap textify drop].freeze + + EMPTY_STACK = [].freeze + private_constant :EMPTY_STACK + + # Containers that hold inline content only: a link's text (CommonMark + # §6.3), and emphasis and heading content. + INLINE_CONTAINERS = [ + AST::Url, + AST::Bold, + AST::Italic, + AST::Strikethrough, + AST::Underline, + AST::Superscript, + AST::Subscript, + AST::Heading, + ].freeze + + # AST nodes the Discourse renderer prints as block-level Markdown (their + # output has blank lines around it). One inside an inline container breaks + # that container, so it is moved out. Spoiler and single-line Code stay + # inline and are not listed (Code is handled by {KEEP_INLINE_CODE}). + BLOCK_NODES = [ + AST::Quote, + AST::Heading, + AST::List, + AST::ListItem, + AST::Table, + AST::TableRow, + AST::TableCell, + AST::Details, + AST::Paragraph, + AST::HorizontalRule, + AST::Align, + AST::Poll, + AST::Event, + ].freeze + + # A code span may stay inside an inline container while it is on one line. + # A fenced or multi-line block is moved out. This matches + # +RenderingInterface#block_context?+: Code prints as a fenced block when a + # Text child has a newline (the language alone does not make it a block). + KEEP_INLINE_CODE = + lambda do |_boundary, node| + block = node.children.any? { |c| c.instance_of?(AST::Text) && c.text.include?("\n") } + block ? :hoist_after : :keep + end + + class << self + # A fresh, customizable normalizer with the default rules. Add more with + # {#rule}. + # @return [Normalizer] + def default + new(build_rules) + end + + # The default normalizer, built once and frozen, reused across + # conversions. +#normalize+ and +#violations+ keep no state on the + # instance, so one frozen instance is safe to reuse, also across threads. + # @return [Normalizer] the same frozen instance on every call + def shared_default + @shared_default ||= default.freeze + end + + private + + def build_rules + rules = RuleSet.new + + # §6.3 A link may not contain another link, at any depth. Unwrap the + # inner link and keep its text. + rules.add(parent: AST::Url, child: AST::Url, strategy: :unwrap) + + INLINE_CONTAINERS.each do |container| + rules.add(parent: container, child: AST::Code, strategy: KEEP_INLINE_CODE) + + BLOCK_NODES.each do |block| + next if container == block + + rules.add(parent: container, child: block, strategy: :hoist_after) + end + end + + rules + end + end + + # @param rule_set [RuleSet] + def initialize(rule_set) + @rules = rule_set + end + + # Add or override a rule. Chainable. A rule for a +(parent, child)+ pair + # that already exists is replaced. Raises on a frozen ({shared_default}) + # instance; build a fresh one with {.default}. + # + # @param parent [Class] ancestor AST class + # @param child [Class] contained AST class + # @param strategy [Symbol, #call] one of {STRATEGIES} or a callable + # @return [self] + def rule(parent:, child:, strategy:) + @rules.add(parent:, child:, strategy:) + self + end + + # Rewrite +ast+ in place so it satisfies the rules. + # + # @param ast [AST::Document, AST::Element] + # @return [Array] a report of what changed (empty when nothing did), + # one +{parent:, child:, strategy:, count:}+ row per distinct change. + def normalize(ast) + report = Report.new + Walker.new(@rules, report).call(ast) + report.to_a + end + + # List the violations in +ast+ without changing it. + # + # @param ast [AST::Document, AST::Element] + # @return [Array] +{parent:, child:, strategy:}+ per occurrence + def violations(ast) + found = [] + collect_violations(ast, EMPTY_STACK, found) + found + end + + def freeze + @rules.freeze + super + end + + private + + def collect_violations(element, ancestors, found) + stack = ancestors + [element] + element.children.each do |child| + strategy, boundary = @rules.resolve(child, stack) + strategy = strategy.call(boundary, child) if strategy.respond_to?(:call) + unless strategy.nil? || strategy == :keep + found << { parent: demodulize(boundary.class), child: demodulize(child.class), strategy: } + end + collect_violations(child, stack, found) if child.is_a?(AST::Element) + end + end + + def demodulize(klass) + klass.name.split("::").last + end + end +end diff --git a/lib/markbridge/normalizer/report.rb b/lib/markbridge/normalizer/report.rb new file mode 100644 index 00000000..4f34e9a7 --- /dev/null +++ b/lib/markbridge/normalizer/report.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Markbridge + class Normalizer + # Tally of the transformations a single {Normalizer#normalize} pass + # applied. Held as a local per call (never on the Normalizer), so a + # frozen shared instance stays reusable and thread-safe. + class Report + def initialize + @counts = Hash.new(0) + end + + # @param parent_class [Class] the offending ancestor's class + # @param child_class [Class] the moved/removed node's class + # @param strategy [Symbol] the strategy actually applied + def record(parent_class, child_class, strategy) + @counts[[demodulize(parent_class), demodulize(child_class), strategy]] += 1 + end + + # @return [Boolean] + def empty? + @counts.empty? + end + + # One +{parent:, child:, strategy:, count:}+ row per distinct + # transformation, e.g. + # +{parent: "Url", child: "Image", strategy: :hoist_after, count: 3}+. + # + # @return [Array] + def to_a + @counts.map { |(parent, child, strategy), count| { parent:, child:, strategy:, count: } } + end + + private + + def demodulize(klass) + klass.name.split("::").last + end + end + end +end diff --git a/lib/markbridge/normalizer/rule_set.rb b/lib/markbridge/normalizer/rule_set.rb new file mode 100644 index 00000000..c18222e6 --- /dev/null +++ b/lib/markbridge/normalizer/rule_set.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +module Markbridge + class Normalizer + # Maps a node and its ancestor stack to a strategy. + # + # Matching is exact-class (equivalent to +instance_of?+): rules are + # keyed by +Class+ and looked up via +node.class+, so an anonymous + # +Class.new(AST::Element)+ or any future subclass never accidentally + # matches a rule written for the base class. Registering a rule for a + # +(parent, child)+ pair that already has one replaces it, so later + # layers (Discourse, a consumer's +#rule+) override earlier ones. + class RuleSet + NO_MATCH = [nil, nil].freeze + + def initialize + @by_parent = {} # parent_class => { child_class => strategy } + @child_classes = Set.new + end + + # Register (or replace) a rule. + # + # @param parent [Class] ancestor AST class + # @param child [Class] contained AST class + # @param strategy [Symbol, #call] a strategy symbol or callable + # @return [self] + def add(parent:, child:, strategy:) + validate_strategy!(strategy) + (@by_parent[parent] ||= {})[child] = strategy + @child_classes << child + self + end + + # Resolve the strategy for +child+ given its ancestor stack (root + # first). Returns +[strategy, boundary]+ where +boundary+ is the + # *outermost* ancestor whose class has a rule for +child+'s class, or + # {NO_MATCH} (+[nil, nil]+) when nothing matches. + # + # @param child [AST::Node] + # @param ancestors [Array] root-first ancestor stack + # @return [Array(Object, AST::Element), Array(nil, nil)] + def resolve(child, ancestors) + child_class = child.class + # Skip the ancestor scan for a class no rule targets (most nodes, for + # example plain text). The scan below returns the same result for such + # a class, so this only saves work. + return NO_MATCH unless @child_classes.include?(child_class) + + scan_ancestors(child_class, ancestors) + end + + # Freeze so a shared instance raises if something tries to change it. + # Freezing +@by_parent+ and its inner hashes is enough: {#add} writes + # there before it touches +@child_classes+, so a frozen instance raises + # on the +@by_parent+ write first. +@child_classes+ is never reached, so + # it does not need freezing. + def freeze + @by_parent.each_value(&:freeze) + @by_parent.freeze + super + end + + private + + # The ancestor scan behind {#resolve}: the outermost matching ancestor + # wins. It is split from the skip check in {#resolve} so the scan can be + # tested on its own. + def scan_ancestors(child_class, ancestors) + ancestors.each do |ancestor| + strategies = @by_parent[ancestor.class] + next unless strategies + + strategy = strategies[child_class] + return strategy, ancestor if strategy + end + NO_MATCH + end + + def validate_strategy!(strategy) + return if strategy.respond_to?(:call) + return if STRATEGIES.include?(strategy) + + raise ArgumentError, + "unknown strategy #{strategy.inspect} " \ + "(expected one of #{STRATEGIES.inspect} or a callable)" + end + end + end +end diff --git a/lib/markbridge/normalizer/text_projection.rb b/lib/markbridge/normalizer/text_projection.rb new file mode 100644 index 00000000..7d6552c8 --- /dev/null +++ b/lib/markbridge/normalizer/text_projection.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Markbridge + class Normalizer + # Best-effort plain-text projection of a subtree, used by the + # +:textify+ strategy. Concatenates text content; renders a {AST::Mention} + # as its literal +@name+, and opaque leaf nodes as their alt/raw text + # when they carry one, otherwise the empty string. + module TextProjection + class << self + # @param node [AST::Node] + # @return [String] + def call(node) + case node + when AST::Text, AST::MarkdownText + node.text + when AST::Mention + "@#{node.name}" + when AST::Element + node.children.map { |child| call(child) }.join + else + leaf_text(node) + end + end + + # @param node [AST::Node] an opaque leaf (Upload, Attachment, …) + # @return [String] + def leaf_text(node) + return node.alt if node.respond_to?(:alt) && node.alt + return node.raw if node.respond_to?(:raw) && node.raw + + "" + end + end + end + end +end diff --git a/lib/markbridge/normalizer/walker.rb b/lib/markbridge/normalizer/walker.rb new file mode 100644 index 00000000..fb993652 --- /dev/null +++ b/lib/markbridge/normalizer/walker.rb @@ -0,0 +1,250 @@ +# frozen_string_literal: true + +module Markbridge + class Normalizer + # The parent-aware tree-rewriting engine. One {Walker} is built per + # {Normalizer#normalize} call (holding that call's {RuleSet} and + # {Report}; no state is kept on the Normalizer). It changes the tree + # in place via {AST::Element#replace_children}, touching only elements + # whose children actually changed. + # + # The main rule is resolve-before-descend: a child's strategy is resolved + # against its current ancestor stack first, and a moved subtree (hoist or + # unwrap) is then walked against the ancestor stack it will have in its + # new place. So a node that leaves a link does not see the link while its + # own inside is normalized. That keeps a legally nested quote-in-quote or + # image-in-quote intact, and it means a second normalize reports nothing. + # + # Hoisting: a node taken out of an inline container is moved up, tagged + # with its boundary (the outermost matching ancestor, compared by + # identity), and placed right after that boundary, at the boundary's + # parent. A wrapper left empty by a hoist or drop is removed (see + # {PRUNE_WHEN_EMPTY}), so no empty +**+ +**+ markers stay. + # + # For speed, the ancestor stack is one shared array that is pushed and + # popped, and an element's children list is copied only when a child + # first changes (copy-on-write). A subtree with no violations allocates + # nothing and is left as it was, so the pass can run by default. + class Walker + EMPTY = [].freeze + + # Wrappers that mean nothing once they are empty, so an empty one is + # removed instead of kept. +AST::Url+ is not here on purpose: an empty + # link still renders as a bare URL, so a hoist that empties a link keeps + # the link. + PRUNE_WHEN_EMPTY = [ + AST::Bold, + AST::Italic, + AST::Underline, + AST::Strikethrough, + AST::Superscript, + AST::Subscript, + AST::Color, + AST::Size, + AST::Align, + AST::Email, + ].freeze + + # @param rule_set [RuleSet] + # @param report [Report] + def initialize(rule_set, report) + @rules = rule_set + @report = report + end + + # Normalize +document+'s subtree in place. + # @param document [AST::Document] + def call(document) + _element, bubble = normalize_element(document, []) + # Defensive: anything that never reached its boundary becomes a + # trailing sibling rather than being lost. Well-formed rule tables + # do not get here. + bubble.each { |node, _boundary| document << node } + end + + private + + # Copy-on-write check: a kept child that came back as the same object + # (so normalizing it changed nothing) with no bubble, while nothing + # earlier changed, needs no rebuilt +out+. This only saves work. If it + # is wrong, the code still rebuilds the same child list, so its + # mutations do not change the output. + def unchanged?(out, normalized, child, child_bubble) + out.nil? && normalized.equal?(child) && child_bubble.empty? + end + + # Normalize a node's descendants. Elements recurse; leaves are + # returned untouched. Returns +[node_or_nil, bubble]+ — +nil+ when the + # element was pruned, and +bubble+ is the list of +[node, boundary]+ + # pairs to hoist above this node. + def normalize_node(node, stack) + return node, EMPTY unless node.is_a?(AST::Element) + + normalize_element(node, stack) + end + + # +stack+ is a shared, mutable ancestor stack (root first): +element+ + # is pushed for the duration of its children's processing and popped + # after. +out+ (the rebuilt child list) and +bubble+ stay +nil+ until + # a child actually changes, so the clean path allocates nothing. + def normalize_element(element, stack) + stack.push(element) + children = element.children + out = nil + bubble = nil + + children.each_with_index do |child, index| + strategy, boundary = @rules.resolve(child, stack) + strategy = strategy.call(boundary, child) if strategy.respond_to?(:call) + + if strategy.nil? || strategy == :keep + normalized, child_bubble = normalize_node(child, stack) + next if unchanged?(out, normalized, child, child_bubble) + + out ||= children[0, index] + bubble = append_kept(normalized, child_bubble, child, out, bubble) + else + out ||= children[0, index] + bubble = emit(child, strategy, boundary, stack, out, bubble) + end + end + + stack.pop + element.replace_children(coalesce(out)) if out + + raised = bubble || EMPTY + return nil, raised if prune?(element, out, children) + + [element, raised] + end + + # An element is removed when it ends up with no children and is one of + # the wrappers in {PRUNE_WHEN_EMPTY}. + def prune?(element, out, children) + empty = out ? out.empty? : children.empty? + empty && PRUNE_WHEN_EMPTY.include?(element.class) + end + + # Resolve one child (against +stack+) and place it into an existing + # +out+ — the shared entry point used by {#emit}'s +:unwrap+ recursion, + # where +out+ already exists. + def resolve_into(child, stack, out, bubble) + strategy, boundary = @rules.resolve(child, stack) + strategy = strategy.call(boundary, child) if strategy.respond_to?(:call) + + if strategy.nil? || strategy == :keep + child2, child_bubble = normalize_node(child, stack) + append_kept(child2, child_bubble, child, out, bubble) + else + emit(child, strategy, boundary, stack, out, bubble) + end + end + + # Append a kept child that is already normalized, then place any of its + # bubbles whose boundary is this child. ({#land} does nothing when + # +child_bubble+ is empty, so there is no separate check for that.) + def append_kept(normalized, child_bubble, child, out, bubble) + out << normalized unless normalized.nil? + land(child_bubble, child, out, bubble) + end + + # Apply a non-keep strategy for +child+, appending to +out+ and + # returning the (possibly newly allocated) +bubble+. + def emit(child, strategy, boundary, stack, out, bubble) + case strategy + when :hoist_after + hoist(child, boundary, stack, bubble) + when :unwrap + unwrap(child, boundary, stack, out, bubble) + when :textify + @report.record(boundary.class, child.class, :textify) + out << AST::Text.new(TextProjection.call(child)) + bubble + when :drop + @report.record(boundary.class, child.class, :drop) + bubble + when Array + # A callable returned replacement nodes to splice in place. + @report.record(boundary.class, child.class, :replace) + strategy.each { |node| out << node } + bubble + else + raise ArgumentError, "strategy resolved to #{strategy.inspect}" + end + end + + def hoist(child, boundary, stack, bubble) + @report.record(boundary.class, child.class, :hoist_after) + # Walk the relocated subtree against its destination stack (the + # ancestors strictly above the boundary) so its interior never sees + # the boundary it is leaving. + child2, child_bubble = normalize_node(child, ancestors_above(boundary, stack)) + bubble ||= [] + bubble << [child2, boundary] unless child2.nil? + # For the built-in tables a hoisted subtree yields no escaping + # bubbles; carry any (from a custom rule) up as a best effort. + child_bubble.each { |entry| bubble << entry } + bubble + end + + def unwrap(child, boundary, stack, out, bubble) + # Unwrap means "promote the element's children"; a leaf has none. A + # rule that targets one is a misconfiguration, so keep the node in + # place rather than silently dropping it (and don't report a no-op). + unless child.is_a?(AST::Element) + out << child + return bubble + end + + @report.record(boundary.class, child.class, :unwrap) + # Unwrap: run the child's children through the current +out+, resolved + # against the current stack. Resolving them again in the same pass is + # what fixes deeply nested links in one go. + child.children.each { |grandchild| bubble = resolve_into(grandchild, stack, out, bubble) } + bubble + end + + # Land inbound bubbles whose boundary is +child+ (this level is the + # boundary's parent), each after the previous to preserve order; + # propagate the rest upward. + def land(child_bubble, child, out, bubble) + child_bubble.each do |node, boundary| + if boundary.equal?(child) + out << node + else + bubble ||= [] + bubble << [node, boundary] + end + end + bubble + end + + # Ancestors strictly above +boundary+ in +stack+ — the stack the + # hoisted node inherits (its parent becomes the boundary's parent). + def ancestors_above(boundary, stack) + index = stack.index { |ancestor| ancestor.equal?(boundary) } + stack.first(index) + end + + # Coalesce adjacent text (textify can create neighbours that +#<<+ + # would have merged) just before committing a changed child list. + def coalesce(nodes) + nodes.each_with_object([]) do |node, acc| + last = acc.last + # mergeable? is false when +last+ is nil (the first node), so no + # separate nil-guard is needed. + if mergeable?(last, node) + last.merge(node) + else + acc << node + end + end + end + + def mergeable?(left, right) + (left.instance_of?(AST::Text) && right.instance_of?(AST::Text)) || + (left.instance_of?(AST::MarkdownText) && right.instance_of?(AST::MarkdownText)) + end + end + end +end diff --git a/lib/markbridge/renderers/discourse/tags/event_tag.rb b/lib/markbridge/renderers/discourse/tags/event_tag.rb index 070202be..2e586a07 100644 --- a/lib/markbridge/renderers/discourse/tags/event_tag.rb +++ b/lib/markbridge/renderers/discourse/tags/event_tag.rb @@ -18,11 +18,9 @@ module Tags # end # end class EventTag < Tag - def render(element, interface) + def render(element, _interface) body = element.raw || build_event_bbcode(element) - return "\n\n#{body}\n\n" if interface.html_mode? - - "#{body}\n\n" + "\n\n#{body}\n\n" end private diff --git a/lib/markbridge/renderers/discourse/tags/poll_tag.rb b/lib/markbridge/renderers/discourse/tags/poll_tag.rb index 560e1b27..fd5c3359 100644 --- a/lib/markbridge/renderers/discourse/tags/poll_tag.rb +++ b/lib/markbridge/renderers/discourse/tags/poll_tag.rb @@ -18,11 +18,9 @@ module Tags # end # end class PollTag < Tag - def render(element, interface) + def render(element, _interface) body = element.raw || build_poll_bbcode(element) - return "\n\n#{body}\n\n" if interface.html_mode? - - "#{body}\n\n" + "\n\n#{body}\n\n" end private diff --git a/mutant.yml b/mutant.yml index 89133131..eaf41335 100644 --- a/mutant.yml +++ b/mutant.yml @@ -282,6 +282,21 @@ matcher: # path runs only for the Document branch. - Markbridge::Parsers::HTML::Parser#parse + # Normalizer fast-path guards. Both methods exist ONLY to skip work; + # when a mutation defeats the guard, the fallback recomputes the same + # result, so every surviving mutation is output-equivalent (byte-for- + # byte). The real behaviour they gate is pinned elsewhere: + # RuleSet#resolve — the ancestor scan is extracted into + # #scan_ancestors, which the resolve/normalize specs cover; the + # guard only decides whether to call it, and a non-candidate class + # yields NO_MATCH from the scan anyway. + # Walker#unchanged? — the copy-on-write skip; when it wrongly returns + # false, normalize_element just rebuilds an identical child list + # (the "leaves a violation-free tree untouched" and COW specs pin + # the observable behaviour). + - Markbridge::Normalizer::RuleSet#resolve + - Markbridge::Normalizer::Walker#unchanged? + mutation: ignore_patterns: # Bucket A bound-check equivalences. @current_pos, pos, and diff --git a/spec/integration/markbridge/normalizer_spec.rb b/spec/integration/markbridge/normalizer_spec.rb new file mode 100644 index 00000000..ca967a8c --- /dev/null +++ b/spec/integration/markbridge/normalizer_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +# End-to-end: the default normalization runs between parse and render, so +# these assertions show the raw Markdown you get. The default fixes legality +# (a link inside a link, a block inside an inline container). Discourse policy +# like moving an image out of a link is a rule the consumer adds. +RSpec.describe "Markbridge normalization (end-to-end)" do + def convert(input, **kwargs) = Markbridge.convert(input, format: :bbcode, **kwargs) + + describe "default: a link inside a link" do + let(:input) { "[url=https://a.com][url=https://b.com]click[/url][/url]" } + + it "collapses to a single link and reports it" do + conv = convert(input) + + expect(conv.markdown).to eq("[click](https://a.com)") + expect(conv.diagnostics[:normalization]).to contain_exactly( + { parent: "Url", child: "Url", strategy: :unwrap, count: 1 }, + ) + end + + it "leaves the tree alone when normalize: false" do + expect(convert(input, normalize: false).diagnostics[:normalization]).to be_nil + end + end + + describe "an image inside a link" do + let(:input) { "[url=https://ex.com][img]https://ex.com/i.png[/img][/url]" } + + it "is left as a linked image by default (not a default rule)" do + expect(convert(input).markdown).to eq("[![](https://ex.com/i.png)](https://ex.com)") + expect(convert(input).diagnostics[:normalization]).to be_nil + end + + it "is hoisted out by a consumer rule (how migrations-tooling does it)" do + normalizer = Markbridge::Normalizer.default + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + + conv = convert(input, normalize: normalizer) + expect(conv.markdown).not_to include("[![") # not a linked image + expect(conv.markdown).to include("![](https://ex.com/i.png)") + expect(conv.diagnostics[:normalization]).to contain_exactly( + { parent: "Url", child: "Image", strategy: :hoist_after, count: 1 }, + ) + end + end + + describe "clean content" do + it "is untouched and reports nothing" do + conv = convert("[b]hello[/b] and [i]world[/i]") + + expect(conv.markdown).to eq("**hello** and *world*") + expect(conv.diagnostics[:normalization]).to be_nil + end + end + + describe "validation property: the default leaves no default violation" do + inputs = [ + "[url=https://a.com][url=https://b.com]x[/url][/url]", + "plain text with [b]bold[/b] and a [url=https://ex.com]link[/url]", + "[quote]a quote[/quote] then [url=https://ex.com]link[/url]", + ] + + inputs.each do |input| + it "leaves no violation for #{input.inspect}" do + ast = Markbridge.parse_bbcode(input).ast + Markbridge::Normalizer.shared_default.normalize(ast) + + expect(Markbridge::Normalizer.default.violations(ast)).to eq([]) + end + end + end +end diff --git a/spec/markbridge_spec.rb b/spec/markbridge_spec.rb index 1ee5d88e..0579978b 100644 --- a/spec/markbridge_spec.rb +++ b/spec/markbridge_spec.rb @@ -1,6 +1,27 @@ # frozen_string_literal: true RSpec.describe Markbridge do + # Append a link inside a link — a default violation (the inner link is + # unwrapped) — so a test can prove the +normalize:+ pass ran, whatever the + # source format. + def append_nested_link(ast) + outer = Markbridge::AST::Url.new(href: "https://a.example.com") + inner = Markbridge::AST::Url.new(href: "https://b.example.com") + inner << Markbridge::AST::Text.new("x") + outer << inner + ast << outer + end + + # Append a link wrapping an image. Moving it out is not a default rule, so + # only a normalizer that adds the rule changes this. + def append_linked_image(ast) + url = Markbridge::AST::Url.new(href: "https://example.com") + url << Markbridge::AST::Image.new(src: "https://example.com/pic.png") + ast << url + end + + UNWRAPPED = [{ parent: "Url", child: "Url", strategy: :unwrap, count: 1 }].freeze + it "has a version number" do expect(Markbridge::VERSION).not_to be_nil end @@ -66,6 +87,19 @@ end describe ".bbcode_to_markdown" do + it "normalizes the AST by default" do + conversion = described_class.bbcode_to_markdown("hi") { |ast| append_nested_link(ast) } + + expect(conversion.diagnostics[:normalization]).to eq(UNWRAPPED) + end + + it "does not normalize when normalize: false" do + conversion = + described_class.bbcode_to_markdown("hi", normalize: false) { |ast| append_nested_link(ast) } + + expect(conversion.diagnostics[:normalization]).to be_nil + end + it "returns a Conversion whose markdown reflects the input" do result = described_class.bbcode_to_markdown("[b]hi[/b]") @@ -205,6 +239,21 @@ def render(_e, _i) end describe ".html_to_markdown" do + it "normalizes the AST by default" do + conversion = described_class.html_to_markdown("hi") { |ast| append_nested_link(ast) } + + expect(conversion.diagnostics[:normalization]).to eq(UNWRAPPED) + end + + it "does not normalize when normalize: false" do + conversion = + described_class.html_to_markdown("hi", normalize: false) do |ast| + append_nested_link(ast) + end + + expect(conversion.diagnostics[:normalization]).to be_nil + end + it "renders HTML to a Conversion" do result = described_class.html_to_markdown("hi") @@ -304,6 +353,22 @@ def render(_e, _i) describe ".text_formatter_xml_to_markdown" do let(:xml) { "[b]hi[/b]" } + it "normalizes the AST by default" do + conversion = + described_class.text_formatter_xml_to_markdown(xml) { |ast| append_nested_link(ast) } + + expect(conversion.diagnostics[:normalization]).to eq(UNWRAPPED) + end + + it "does not normalize when normalize: false" do + conversion = + described_class.text_formatter_xml_to_markdown(xml, normalize: false) do |ast| + append_nested_link(ast) + end + + expect(conversion.diagnostics[:normalization]).to be_nil + end + it "renders TextFormatter XML to a Conversion" do result = described_class.text_formatter_xml_to_markdown(xml) @@ -393,6 +458,22 @@ def render(_e, _i) end describe ".mediawiki_to_markdown" do + it "normalizes the AST by default" do + conversion = + described_class.mediawiki_to_markdown("'''hi'''") { |ast| append_nested_link(ast) } + + expect(conversion.diagnostics[:normalization]).to eq(UNWRAPPED) + end + + it "does not normalize when normalize: false" do + conversion = + described_class.mediawiki_to_markdown("'''hi'''", normalize: false) do |ast| + append_nested_link(ast) + end + + expect(conversion.diagnostics[:normalization]).to be_nil + end + it "renders MediaWiki to a Conversion" do expect(described_class.mediawiki_to_markdown("'''hi'''").markdown).to eq("**hi**") end @@ -616,6 +697,38 @@ def render(_e, _i) end describe ".render" do + it "normalizes the AST by default" do + ast = Markbridge::AST::Document.new + append_nested_link(ast) + + expect(described_class.render(ast).diagnostics[:normalization]).to eq(UNWRAPPED) + end + + it "does not normalize when normalize: false" do + ast = Markbridge::AST::Document.new + append_nested_link(ast) + + expect(described_class.render(ast, normalize: false).diagnostics[:normalization]).to be_nil + end + + it "uses a passed Normalizer, including a subclass instance (is_a?, not instance_of?)" do + subclass = Class.new(Markbridge::Normalizer) + normalizer = subclass.default + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :drop) + ast = Markbridge::AST::Document.new + append_linked_image(ast) + + conversion = described_class.render(ast, normalize: normalizer) + + # The subclass instance is accepted and its :drop rule runs. With + # instance_of? it would be rejected, shared_default would run instead, + # and shared_default has no image rule, so the image would stay. + expect(conversion.diagnostics[:normalization]).to eq( + [{ parent: "Url", child: "Image", strategy: :drop, count: 1 }], + ) + expect(conversion.ast.descendants(Markbridge::AST::Image)).to be_empty + end + it "renders a Document AST through the default Discourse renderer" do doc = described_class.parse_bbcode("[b]hi[/b]").ast diff --git a/spec/unit/markbridge/ast/element_spec.rb b/spec/unit/markbridge/ast/element_spec.rb index a4e147a1..e4d5dd0b 100644 --- a/spec/unit/markbridge/ast/element_spec.rb +++ b/spec/unit/markbridge/ast/element_spec.rb @@ -272,4 +272,48 @@ ) end end + + describe "#replace_children" do + let(:element) { test_element_class.new } + + before { element << Markbridge::AST::Text.new("old") } + + it "swaps the entire child list" do + replacement = [Markbridge::AST::Bold.new, Markbridge::AST::Italic.new] + element.replace_children(replacement) + + expect(element.children).to eq(replacement) + end + + it "returns self for chaining" do + expect(element.replace_children([])).to be(element) + end + + it "accepts an empty list" do + element.replace_children([]) + expect(element.children).to eq([]) + end + + it "does NOT merge adjacent Text nodes (the merge invariant is #<< only)" do + first = Markbridge::AST::Text.new("hello") + second = Markbridge::AST::Text.new(" world") + element.replace_children([first, second]) + + expect(element.children).to eq([first, second]) + end + + it "raises TypeError when any entry is not a Node, naming the offending value" do + expect { element.replace_children([Markbridge::AST::Bold.new, "nope"]) }.to raise_error( + TypeError, + /replace_children on #{Regexp.escape(test_element_class.to_s)} expected Markbridge::AST::Nodes, got String/, + ) + end + + it "raises TypeError for a nil entry" do + expect { element.replace_children([nil]) }.to raise_error( + TypeError, + /replace_children on #{Regexp.escape(test_element_class.to_s)} expected Markbridge::AST::Nodes, got nil/, + ) + end + end end diff --git a/spec/unit/markbridge/normalizer/report_spec.rb b/spec/unit/markbridge/normalizer/report_spec.rb new file mode 100644 index 00000000..4137a916 --- /dev/null +++ b/spec/unit/markbridge/normalizer/report_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +RSpec.describe Markbridge::Normalizer::Report do + subject(:report) { described_class.new } + + describe "#empty?" do + it "is true for a fresh report" do + expect(report.empty?).to be(true) + end + + it "is false after a record" do + report.record(Markbridge::AST::Url, Markbridge::AST::Image, :hoist_after) + expect(report.empty?).to be(false) + end + end + + describe "#to_a" do + it "is empty for a fresh report" do + expect(report.to_a).to eq([]) + end + + it "returns one row per transformation with demodulized class names" do + report.record(Markbridge::AST::Url, Markbridge::AST::Image, :hoist_after) + expect(report.to_a).to eq( + [{ parent: "Url", child: "Image", strategy: :hoist_after, count: 1 }], + ) + end + + it "tallies repeats of the same transformation" do + 3.times { report.record(Markbridge::AST::Url, Markbridge::AST::Image, :hoist_after) } + expect(report.to_a).to eq( + [{ parent: "Url", child: "Image", strategy: :hoist_after, count: 3 }], + ) + end + + it "keeps transformations that differ in parent or child as separate rows" do + report.record(Markbridge::AST::Url, Markbridge::AST::Image, :hoist_after) + report.record(Markbridge::AST::Url, Markbridge::AST::Url, :unwrap) + expect(report.to_a).to contain_exactly( + { parent: "Url", child: "Image", strategy: :hoist_after, count: 1 }, + { parent: "Url", child: "Url", strategy: :unwrap, count: 1 }, + ) + end + + it "distinguishes rows that differ only by strategy" do + report.record(Markbridge::AST::Url, Markbridge::AST::Image, :hoist_after) + report.record(Markbridge::AST::Url, Markbridge::AST::Image, :drop) + expect(report.to_a.size).to eq(2) + end + end +end diff --git a/spec/unit/markbridge/normalizer/rule_set_spec.rb b/spec/unit/markbridge/normalizer/rule_set_spec.rb new file mode 100644 index 00000000..aa7b30fb --- /dev/null +++ b/spec/unit/markbridge/normalizer/rule_set_spec.rb @@ -0,0 +1,199 @@ +# frozen_string_literal: true + +RSpec.describe Markbridge::Normalizer::RuleSet do + subject(:rule_set) { described_class.new } + + describe "#resolve" do + it "returns [nil, nil] when the child class is not in any rule" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + bold = Markbridge::AST::Bold.new + expect(rule_set.resolve(bold, [Markbridge::AST::Url.new])).to eq([nil, nil]) + end + + it "returns [strategy, boundary] for the matching ancestor" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + url = Markbridge::AST::Url.new + image = Markbridge::AST::Image.new + + strategy, boundary = rule_set.resolve(image, [url]) + expect(strategy).to eq(:hoist_after) + expect(boundary).to be(url) + end + + it "picks the OUTERMOST matching ancestor when several match" do + rule_set.add(parent: Markbridge::AST::Url, child: Markbridge::AST::Url, strategy: :unwrap) + outer = Markbridge::AST::Url.new + inner = Markbridge::AST::Url.new + target = Markbridge::AST::Url.new + + _strategy, boundary = rule_set.resolve(target, [outer, inner]) + expect(boundary).to be(outer) + end + + it "matches by exact class, not is_a? — an anonymous subclass does not match" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + subclass_image = Class.new(Markbridge::AST::Image).new(src: "x") + + expect(rule_set.resolve(subclass_image, [Markbridge::AST::Url.new])).to eq([nil, nil]) + end + + it "does not match an anonymous subclass of the parent either" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + subclass_url = Class.new(Markbridge::AST::Url).new + + expect(rule_set.resolve(Markbridge::AST::Image.new, [subclass_url])).to eq([nil, nil]) + end + + it "skips an ancestor whose class has no rules at all" do + rule_set.add(parent: Markbridge::AST::Bold, child: Markbridge::AST::Image, strategy: :drop) + bold = Markbridge::AST::Bold.new + + # Url has no rules; Bold does — resolution must not stop at Url. + strategy, boundary = + rule_set.resolve(Markbridge::AST::Image.new, [Markbridge::AST::Url.new, bold]) + expect(strategy).to eq(:drop) + expect(boundary).to be(bold) + end + + it "skips an ancestor that has rules but none for this child class" do + rule_set.add(parent: Markbridge::AST::Url, child: Markbridge::AST::Url, strategy: :unwrap) + rule_set.add(parent: Markbridge::AST::Bold, child: Markbridge::AST::Image, strategy: :drop) + bold = Markbridge::AST::Bold.new + + # Url has a rules hash, but not one for Image — must fall through to Bold. + strategy, boundary = + rule_set.resolve(Markbridge::AST::Image.new, [Markbridge::AST::Url.new, bold]) + expect(strategy).to eq(:drop) + expect(boundary).to be(bold) + end + end + + describe "#add override" do + it "replaces an earlier rule for the same (parent, child) pair" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + rule_set.add(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :drop) + + strategy, = rule_set.resolve(Markbridge::AST::Image.new, [Markbridge::AST::Url.new]) + expect(strategy).to eq(:drop) + end + + it "returns self for chaining" do + expect( + rule_set.add(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :drop), + ).to be(rule_set) + end + + it "accepts a callable strategy" do + callable = ->(_boundary, _node) { :keep } + expect { + rule_set.add(parent: Markbridge::AST::Url, child: Markbridge::AST::Code, strategy: callable) + }.not_to raise_error + end + + it "raises for an unknown symbol strategy" do + expect { + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :explode, + ) + }.to raise_error(ArgumentError, /unknown strategy :explode/) + end + end + + describe "#freeze" do + it "freezes the receiver (via super)" do + rule_set.freeze + expect(rule_set).to be_frozen + end + + it "makes adding to an EXISTING parent raise (inner hash deep-frozen)" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + rule_set.freeze + + expect { + rule_set.add(parent: Markbridge::AST::Url, child: Markbridge::AST::Url, strategy: :unwrap) + }.to raise_error(FrozenError) + end + + it "makes adding a NEW parent raise (top-level hash frozen)" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + rule_set.freeze + + expect { + rule_set.add(parent: Markbridge::AST::Bold, child: Markbridge::AST::Image, strategy: :drop) + }.to raise_error(FrozenError) + end + + # The raise alone doesn't prove *which* collection stopped the write — + # a later-frozen collection would still raise while an earlier unfrozen + # one silently accepted the rule. These re-add an ALREADY-registered + # child class (Image) so the child-class fast-reject can't mask a sneak, + # then check the strategy did not change. + it "deep-freezes inner hashes so a raised add cannot mutate shared state" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + rule_set.freeze + + begin + rule_set.add(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :drop) + rescue FrozenError + # expected + end + + strategy, = rule_set.resolve(Markbridge::AST::Image.new, [Markbridge::AST::Url.new]) + expect(strategy).to eq(:hoist_after) # unchanged; :drop never sneaked in + end + + it "freezes the top-level hash so a raised new-parent add cannot mutate shared state" do + rule_set.add( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + rule_set.freeze + + begin + rule_set.add(parent: Markbridge::AST::Bold, child: Markbridge::AST::Image, strategy: :drop) + rescue FrozenError + # expected + end + + # No (Bold, *) parent bucket was created before the raise. + expect(rule_set.resolve(Markbridge::AST::Image.new, [Markbridge::AST::Bold.new])).to eq( + [nil, nil], + ) + end + end +end diff --git a/spec/unit/markbridge/normalizer/text_projection_spec.rb b/spec/unit/markbridge/normalizer/text_projection_spec.rb new file mode 100644 index 00000000..5a08544c --- /dev/null +++ b/spec/unit/markbridge/normalizer/text_projection_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +RSpec.describe Markbridge::Normalizer::TextProjection do + def project(node) = described_class.call(node) + + it "returns a Text node's own text" do + expect(project(Markbridge::AST::Text.new("hello"))).to eq("hello") + end + + it "returns a MarkdownText node's own text" do + expect(project(Markbridge::AST::MarkdownText.new("**hi**"))).to eq("**hi**") + end + + it "renders a Mention as its literal @name" do + expect(project(Markbridge::AST::Mention.new(name: "alice"))).to eq("@alice") + end + + it "concatenates the text of an Element's descendants" do + bold = Markbridge::AST::Bold.new + bold << Markbridge::AST::Text.new("a") + bold << Markbridge::AST::Text.new("b") + inner = Markbridge::AST::Italic.new + inner << Markbridge::AST::Text.new("c") + bold << inner + + expect(project(bold)).to eq("abc") + end + + it "uses an opaque leaf's alt text when present" do + upload = Markbridge::AST::Upload.new(sha1: "x", type: :image, alt: "a cat") + expect(project(upload)).to eq("a cat") + end + + it "falls back to raw when there is no alt" do + upload = Markbridge::AST::Upload.new(sha1: "x", type: :image, raw: "![](upload://x)") + expect(project(upload)).to eq("![](upload://x)") + end + + it "projects to the empty string for an opaque leaf carrying no text" do + expect(project(Markbridge::AST::HorizontalRule.new)).to eq("") + end + + it "projects to the empty string when alt and raw are both nil" do + # Upload responds to both :alt and :raw, but here they are nil — the + # guards must fall through to "" rather than returning nil. + expect(project(Markbridge::AST::Upload.new(sha1: "x"))).to eq("") + end +end diff --git a/spec/unit/markbridge/normalizer/walker_spec.rb b/spec/unit/markbridge/normalizer/walker_spec.rb new file mode 100644 index 00000000..8af46442 --- /dev/null +++ b/spec/unit/markbridge/normalizer/walker_spec.rb @@ -0,0 +1,697 @@ +# frozen_string_literal: true + +# The Walker is exercised through the public Normalizer#normalize (no private +# calls), but lives under a describe matching the subject so mutant selects +# these as the engine's tests. +RSpec.describe Markbridge::Normalizer::Walker do + def el(klass, *children, **kwargs) + node = kwargs.empty? ? klass.new : klass.new(**kwargs) + children.each { |child| node << child } + node + end + + def doc(*children) = el(Markbridge::AST::Document, *children) + def text(string) = Markbridge::AST::Text.new(string) + def url(*children, href: "https://ex.com") = el(Markbridge::AST::Url, *children, href:) + def image(src: "https://ex.com/i.png") = Markbridge::AST::Image.new(src:) + + # The default rules do not move an image out of a link. These engine + # examples add that rule so they have a simple violation to use. + let(:normalizer) do + Markbridge::Normalizer.default.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + end + + describe "hoist_after" do + it "moves an image out of a link, reports it, and leaves the empty link" do + img = image + tree = doc(url(img)) + report = normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Image]) + expect(tree.children.last).to be(img) # the same node, moved + expect(tree.children.first.children).to eq([]) + expect(report).to eq([{ parent: "Url", child: "Image", strategy: :hoist_after, count: 1 }]) + end + + it "does not push a bubble entry when the hoisted node is pruned to nil" do + # An empty prunable wrapper hoisted out normalizes to nil — it must not + # be pushed as a [nil, boundary] bubble (which would splice a nil child). + custom = Markbridge::Normalizer.default + custom.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Bold, + strategy: :hoist_after, + ) + tree = doc(url(Markbridge::AST::Bold.new)) # empty bold + + expect { custom.normalize(tree) }.not_to raise_error + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url]) + expect(tree.children.first.children).to eq([]) + end + + it "lands a hoisted node inside the boundary's (non-root) parent, in position" do + img = image + italic = el(Markbridge::AST::Italic, text("after")) + # image in url in bold: image hoists to right after the url, INSIDE the bold + tree = doc(el(Markbridge::AST::Bold, url(img), italic)) + normalizer.normalize(tree) + + bold = tree.children.first + expect(bold).to be_a(Markbridge::AST::Bold) + expect(bold.children.map(&:class)).to eq( + [Markbridge::AST::Url, Markbridge::AST::Image, Markbridge::AST::Italic], + ) + expect(bold.children[1]).to be(img) # landed between url and italic, not at root + end + + it "hoists to the outermost offending ancestor across nested formatting" do + img = image + tree = doc(url(el(Markbridge::AST::Bold, img))) + normalizer.normalize(tree) + + # after the link, not after the bold + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Image]) + expect(tree.children.last).to be(img) + # bold emptied → pruned + expect(tree.descendants(Markbridge::AST::Bold)).to be_empty + end + + it "preserves document order for several hoists to one boundary" do + img1 = Markbridge::AST::Image.new(src: "one") + img2 = Markbridge::AST::Image.new(src: "two") + tree = doc(url(img1, text("mid"), img2)) + normalizer.normalize(tree) + + link, first, second = tree.children + expect(link.children.map(&:text)).to eq(["mid"]) + expect([first, second]).to eq([img1, img2]) # order preserved + end + + it "keeps a relocated subtree's interior intact (destination-stack walk)" do + inner_quote = el(Markbridge::AST::Quote, text("deep")) + tree = doc(url(el(Markbridge::AST::Quote, inner_quote))) + normalizer.normalize(tree) + + hoisted = tree.children.last + expect(hoisted).to be_a(Markbridge::AST::Quote) + expect(hoisted.children).to eq([inner_quote]) # inner quote NOT flattened out + end + + it "hoists a block-level Poll out of an inline container (not just a link)" do + # Poll renders block-level, so it breaks emphasis with blank lines just + # like it breaks a link label. + poll = Markbridge::AST::Poll.new(name: "p") + tree = doc(el(Markbridge::AST::Bold, text("x"), poll)) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Bold, Markbridge::AST::Poll]) + expect(tree.children.last).to be(poll) + expect(tree.children.first.children.map(&:class)).to eq([Markbridge::AST::Text]) + end + + it "does not rip an image out of a quote that is itself hoisted from a link" do + img = image + tree = doc(url(el(Markbridge::AST::Quote, img))) + normalizer.normalize(tree) + + hoisted_quote = tree.children.last + expect(hoisted_quote.children).to eq([img]) # image stays in the quote + end + end + + describe "unwrap" do + it "replaces the inner link with its label, keeping the outer link, and reports it" do + label = text("click") + tree = doc(url(url(label, href: "https://b.com"), href: "https://a.com")) + report = normalizer.normalize(tree) + + outer = tree.children.first + expect(outer.href).to eq("https://a.com") + expect(outer.children).to eq([label]) + expect(report).to eq([{ parent: "Url", child: "Url", strategy: :unwrap, count: 1 }]) + end + + it "re-resolves the dissolved children (image inside inner link hoists out)" do + img = image + inner = url(text("x"), img, href: "https://b.com") + tree = doc(url(inner, href: "https://a.com")) + normalizer.normalize(tree) + + outer, hoisted = tree.children + expect(outer.href).to eq("https://a.com") + expect(outer.children.map(&:class)).to eq([Markbridge::AST::Text]) + expect(hoisted).to be(img) + end + + it "reaches a fixpoint for triple-nested links in one pass" do + tree = doc(url(url(url(text("deep"), href: "c"), href: "b"), href: "a")) + first = normalizer.normalize(tree) + second = normalizer.normalize(tree) + + expect(first).not_to be_empty + expect(second).to eq([]) + expect(tree.children.size).to eq(1) + expect(tree.children.first.href).to eq("a") + end + end + + describe "textify / drop / splice (custom rules)" do + it "textifies a subtree to its plain-text projection" do + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Mention, + strategy: :textify, + ) + tree = doc(url(text("hi "), Markbridge::AST::Mention.new(name: "alice"))) + report = normalizer.normalize(tree) + + link = tree.children.first + expect(link.children.size).to eq(1) + expect(link.children.first.text).to eq("hi @alice") + expect(report).to eq([{ parent: "Url", child: "Mention", strategy: :textify, count: 1 }]) + end + + it "drops a node entirely and reports it" do + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :drop) + tree = doc(url(text("label"), image)) + report = normalizer.normalize(tree) + + expect(tree.children.first.children.map(&:class)).to eq([Markbridge::AST::Text]) + expect(report).to eq([{ parent: "Url", child: "Image", strategy: :drop, count: 1 }]) + end + + it "splices in replacement nodes when a callable returns an Array, reported as :replace" do + replacement = Markbridge::AST::Text.new("[img]") + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: ->(_boundary, _node) { [replacement] }, + ) + tree = doc(url(text("a "), image)) + report = normalizer.normalize(tree) + + # "a " + "[img]" coalesced into one Text + expect(tree.children.first.children.map(&:text)).to eq(["a [img]"]) + expect(report).to eq([{ parent: "Url", child: "Image", strategy: :replace, count: 1 }]) + end + + it "passes the boundary to a callable strategy" do + seen = [] + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: + lambda do |boundary, _node| + seen << boundary + :drop + end, + ) + link = url(image) + tree = doc(link) + normalizer.normalize(tree) + + expect(seen).to eq([link]) # the offending Url, not nil + end + + it "raises when a callable resolves to an unknown strategy" do + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: ->(_boundary, _node) { :bogus }, + ) + tree = doc(url(image)) + + expect { normalizer.normalize(tree) }.to raise_error( + ArgumentError, + /strategy resolved to :bogus/, + ) + end + end + + describe "bubble preservation across non-hoist strategies" do + # A pending hoist (the leading image) must survive when the NEXT sibling + # is handled by a strategy that returns the accumulated bubble. + def link_with(strategy) + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Mention, strategy:) + img = image + tree = doc(url(img, Markbridge::AST::Mention.new(name: "m"))) + normalizer.normalize(tree) + [tree, img] + end + + it "keeps a pending hoist across a :textify sibling" do + tree, img = link_with(:textify) + expect(tree.children.last).to be(img) + end + + it "keeps a pending hoist across a :drop sibling" do + tree, img = link_with(:drop) + expect(tree.children.last).to be(img) + end + + it "keeps a pending hoist across a spliced (callable) sibling" do + tree, img = link_with(->(_b, _n) { [Markbridge::AST::Text.new("m")] }) + expect(tree.children.last).to be(img) + end + + it "keeps a pending hoist across an :unwrap of a leaf sibling" do + tree, img = link_with(:unwrap) # Mention is a leaf → dissolves to nothing + expect(tree.children.last).to be(img) + end + end + + describe "pruning" do + # Every entry in PRUNE_WHEN_EMPTY, emptied via a custom hoist rule so + # even the non-inline-container wrappers (Color/Size/Align/Email) are + # exercised. A missing entry would leave an empty wrapper and fail here. + prunable = { + Markbridge::AST::Bold => { + }, + Markbridge::AST::Italic => { + }, + Markbridge::AST::Underline => { + }, + Markbridge::AST::Strikethrough => { + }, + Markbridge::AST::Superscript => { + }, + Markbridge::AST::Subscript => { + }, + Markbridge::AST::Color => { + color: "red", + }, + Markbridge::AST::Size => { + size: "5", + }, + Markbridge::AST::Align => { + alignment: "center", + }, + Markbridge::AST::Email => { + address: "a@b.c", + }, + } + + prunable.each do |wrapper, kwargs| + it "prunes an emptied #{wrapper.name.split("::").last}" do + custom = Markbridge::Normalizer.default + custom.rule(parent: wrapper, child: Markbridge::AST::Image, strategy: :hoist_after) + node = kwargs.empty? ? wrapper.new : wrapper.new(**kwargs) + node << image + tree = doc(node) + custom.normalize(tree) + + expect(tree.descendants(wrapper)).to be_empty + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Image]) + end + end + + it "keeps an emptied Url (not prunable) as a bare link" do + tree = doc(url(image)) + normalizer.normalize(tree) + + expect(tree.children.first).to be_a(Markbridge::AST::Url) + expect(tree.children.first.children).to eq([]) + end + end + + describe "copy-on-write" do + it "leaves unchanged children as the same objects and only rewrites what moved" do + keep_a = el(Markbridge::AST::Bold, text("a")) + keep_b = el(Markbridge::AST::Italic, text("b")) + img = image + link = url(keep_a, img, keep_b) # image between two non-mergeable wrappers + tree = doc(link) + normalizer.normalize(tree) + + expect(tree.children.first).to be(link) + expect(tree.children.first.children).to eq([keep_a, keep_b]) + expect(tree.children.first.children.first).to be(keep_a) + expect(tree.children.first.children.last).to be(keep_b) + expect(tree.children.last).to be(img) + end + + it "keeps the unchanged prefix when divergence happens after the first child" do + # text at index 0 is kept (no divergence yet); the image at index 1 + # hoists, so the copied prefix must be exactly [text]. + keep = text("keep") + img = image + link = url(keep, img) + tree = doc(link) + normalizer.normalize(tree) + + expect(link.children).to eq([keep]) + expect(link.children.first).to be(keep) + expect(tree.children.last).to be(img) + end + + it "keeps the unchanged prefix when a kept child diverges after the first child" do + # text is kept at index 0 (no divergence); the bold at index 1 is a kept + # child that diverges (its image hoists, emptying it), so the copied + # prefix must be exactly [text] and the emptied bold pruned away. + keep = text("keep") + img = image + link = url(keep, el(Markbridge::AST::Bold, img)) + tree = doc(link) + normalizer.normalize(tree) + + expect(link.children).to eq([keep]) + expect(link.children.first).to be(keep) + expect(tree.children.last).to be(img) + end + + it "pops each element off the shared stack so a later sibling sees no stale ancestor" do + # url_a is a sibling of the bold, not an ancestor of url_b. If url_a were + # left on the stack, url_b would resolve (Url, Url) and be unwrapped. + inner = url(text("b"), href: "https://b.com") + bold = el(Markbridge::AST::Bold, inner) + tree = doc(url(text("a"), href: "https://a.com"), bold) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Bold]) + expect(bold.children.first).to be(inner) # url_b intact, not unwrapped + end + + it "does not touch a violation-free tree at all (same child arrays)" do + inner = text("hello") + bold = el(Markbridge::AST::Bold, inner) + tree = doc(bold) + before = tree.children + report = normalizer.normalize(tree) + + expect(report).to eq([]) + expect(tree.children).to be(before) # array identity: never replaced + expect(bold.children.first).to be(inner) + end + end + + describe "coalescing" do + it "merges adjacent MarkdownText produced by an unwrap" do + md1 = Markbridge::AST::MarkdownText.new("**a**") + md2 = Markbridge::AST::MarkdownText.new("**b**") + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + tree = doc(url(el(Markbridge::AST::Bold, md1, md2))) + normalizer.normalize(tree) + + link = tree.children.first + expect(link.children.size).to eq(1) + expect(link.children.first).to be_a(Markbridge::AST::MarkdownText) + expect(link.children.first.text).to eq("**a****b**") + end + + it "does not merge a Text next to a MarkdownText" do + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + tree = + doc( + url( + el(Markbridge::AST::Bold, text("plain"), Markbridge::AST::MarkdownText.new("**md**")), + ), + ) + normalizer.normalize(tree) + + link = tree.children.first + expect(link.children.map(&:class)).to eq( + [Markbridge::AST::Text, Markbridge::AST::MarkdownText], + ) + end + + it "does not merge a MarkdownText followed by a plain Text" do + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + tree = + doc( + url( + el(Markbridge::AST::Bold, Markbridge::AST::MarkdownText.new("**md**"), text("plain")), + ), + ) + normalizer.normalize(tree) + + expect(tree.children.first.children.map(&:class)).to eq( + [Markbridge::AST::MarkdownText, Markbridge::AST::Text], + ) + end + + it "does not merge (or crash on) a non-text node adjacent to a Text" do + # common_mark has no (Url, Image) rule, so the image survives the unwrap + # and lands next to the text — mergeable? must reject the pair. + unwrapper = Markbridge::Normalizer.default + unwrapper.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + img = image + tree = doc(url(el(Markbridge::AST::Bold, img, text("x")))) + + expect { unwrapper.normalize(tree) }.not_to raise_error + expect(tree.children.first.children.map(&:class)).to eq( + [Markbridge::AST::Image, Markbridge::AST::Text], + ) + end + end + + describe "ancestor stack ordering and destination" do + it "picks the outermost boundary when a node is inside two matching ancestors" do + # quote inside a link inside a heading: (Heading, Quote) and (Url, Quote) + # both match; the outermost (Heading) wins, so the quote lands at the + # document level after the heading — not inside it after the link. + heading = el(Markbridge::AST::Heading, url(el(Markbridge::AST::Quote, text("q"))), level: 1) + tree = doc(heading) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Heading, Markbridge::AST::Quote]) + end + + it "walks a hoisted subtree against the ancestors above its boundary" do + # (Url, Quote) hoists the quote out of the link; a custom (Color, Image) + # rule must still fire on the image inside that quote, which is only + # possible if the destination stack still contains the Color ancestor. + # (Color is used because, unlike an inline container, it has no block + # rule of its own, so the quote's boundary stays at the Url.) + custom = Markbridge::Normalizer.default + custom.rule( + parent: Markbridge::AST::Color, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + img = image + color = el(Markbridge::AST::Color, url(el(Markbridge::AST::Quote, img)), color: "red") + tree = doc(color) + custom.normalize(tree) + + # image escaped the quote entirely (destination stack retained Color) + quote = tree.descendants(Markbridge::AST::Quote).first + expect(quote.descendants(Markbridge::AST::Image)).to be_empty + expect(tree.descendants(Markbridge::AST::Image)).to contain_exactly(img) + end + + it "retains the root in the destination stack (a document-boundary rule fires on deep content)" do + # (Url, Quote) hoists the quote out of the link; a custom (Document, Image) + # rule must still fire on the image inside that quote, which requires the + # destination stack to still contain the Document (root) ancestor. + custom = Markbridge::Normalizer.default + custom.rule( + parent: Markbridge::AST::Document, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + img = image + tree = doc(url(el(Markbridge::AST::Quote, img))) + custom.normalize(tree) + + quote = tree.descendants(Markbridge::AST::Quote).first + expect(quote.descendants(Markbridge::AST::Image)).to be_empty + expect(tree.children.last).to be(img) # escaped all the way to the document + end + + it "lands a propagated hoist after its boundary, not at the very end" do + # image → boundary is the link; a trailing sibling must stay after it. + img = image + tail = el(Markbridge::AST::Italic, text("tail")) + tree = doc(url(el(Markbridge::AST::Bold, img)), tail) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq( + [Markbridge::AST::Url, Markbridge::AST::Image, Markbridge::AST::Italic], + ) + expect(tree.children[1]).to be(img) + end + end + + describe "inline-code predicate through the walker" do + it "keeps an inline code span in a link" do + code = el(Markbridge::AST::Code, text("x")) + tree = doc(url(code)) + normalizer.normalize(tree) + + expect(tree.children.first.children).to eq([code]) + end + + it "hoists a multi-line code block out of a link" do + code = el(Markbridge::AST::Code, text("a\nb")) + tree = doc(url(code)) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Code]) + end + + it "hoists a multi-line code block out of an inline container that is not a link" do + code = el(Markbridge::AST::Code, text("a\nb")) + tree = doc(el(Markbridge::AST::Bold, code)) + normalizer.normalize(tree) + + # bold emptied → pruned; the block lands at the top level + expect(tree.descendants(Markbridge::AST::Bold)).to be_empty + expect(tree.children).to eq([code]) + end + + it "keeps an inline code span inside bold" do + code = el(Markbridge::AST::Code, text("x")) + tree = doc(el(Markbridge::AST::Bold, code)) + normalizer.normalize(tree) + + expect(tree.children.first).to be_a(Markbridge::AST::Bold) + expect(tree.children.first.children).to eq([code]) + end + end + + describe "unwrap edge cases" do + it "keeps a leaf targeted by an unwrap rule and does not report a no-op" do + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Mention, + strategy: :unwrap, + ) + mention = Markbridge::AST::Mention.new(name: "x") + tree = doc(url(text("a"), mention)) + report = normalizer.normalize(tree) + + expect(tree.children.first.children.map(&:class)).to eq( + [Markbridge::AST::Text, Markbridge::AST::Mention], + ) + expect(tree.children.first.children.last).to be(mention) # kept, not dropped + expect(report).to eq([]) # unwrap of a leaf is a silent no-op + end + + it "resolves a callable rule on a dissolved (unwrapped) child, reporting the boundary" do + # unwrap the bold; the code block it held must then be hoisted out of the + # link by the (Url, Code) callable — exercised via the unwrap recursion. + # The reported parent must be the Url boundary, not nil. + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + tree = doc(url(el(Markbridge::AST::Bold, el(Markbridge::AST::Code, text("a\nb"))))) + report = normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Code]) + expect(report).to include({ parent: "Url", child: "Code", strategy: :hoist_after, count: 1 }) + end + + it "passes the real boundary to a callable resolved on a dissolved child" do + seen = [] + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: + lambda do |boundary, _node| + seen << boundary + :drop + end, + ) + link = url(el(Markbridge::AST::Bold, image)) + normalizer.normalize(doc(link)) + + expect(seen).to eq([link]) # the Url boundary, not nil + end + + it "lands a hoist raised inside a dissolved child after that child" do + # (Color, Bold) unwrap dissolves the bold; the inner link keeps its + # image hoist landing right after the link, inside the Color — the + # dissolved child (the link) is itself the boundary. + custom = Markbridge::Normalizer.default + custom.rule(parent: Markbridge::AST::Color, child: Markbridge::AST::Bold, strategy: :unwrap) + custom.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + img = image + color = el(Markbridge::AST::Color, el(Markbridge::AST::Bold, url(img)), color: "red") + custom.normalize(doc(color)) + + expect(color.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Image]) + expect(color.children.last).to be(img) + end + + it "preserves multiple hoists raised while dissolving one child" do + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + img1 = Markbridge::AST::Image.new(src: "one") + img2 = Markbridge::AST::Image.new(src: "two") + tree = doc(url(el(Markbridge::AST::Bold, img1, img2))) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq( + [Markbridge::AST::Url, Markbridge::AST::Image, Markbridge::AST::Image], + ) + expect(tree.children[1, 2]).to eq([img1, img2]) # both images, in order + end + + it "keeps a dissolved child that resolves to :keep (not routed to emit)" do + # unwrap the bold; the mention it held resolves to an explicit :keep and + # must be appended, not sent through emit (which would raise on :keep). + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Mention, + strategy: :keep, + ) + tree = doc(url(el(Markbridge::AST::Bold, Markbridge::AST::Mention.new(name: "x")))) + + expect { normalizer.normalize(tree) }.not_to raise_error + expect(tree.children.first.children.map(&:class)).to eq([Markbridge::AST::Mention]) + end + + it "preserves an earlier sibling's hoist across a later unwrap" do + # image hoists first; then the bold unwraps — the pending [image] bubble + # must survive into the unwrap, not be dropped. + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + img = image + tree = doc(url(img, el(Markbridge::AST::Bold, text("k")))) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Image]) + expect(tree.children.last).to be(img) + expect(tree.children.first.children.map(&:text)).to eq(["k"]) + end + + it "preserves a hoist raised while dissolving, across the remaining children" do + # image (first dissolved child) hoists; the following text must not drop + # its pending bubble. + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Bold, strategy: :unwrap) + img = image + tree = doc(url(el(Markbridge::AST::Bold, img, text("k")))) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Image]) + expect(tree.children.last).to be(img) + expect(tree.children.first.children.map(&:text)).to eq(["k"]) + end + end + + describe "root-level bubble (defensive)" do + it "appends a node hoisted with the document as its boundary" do + # A (Document, Image) rule makes the document the hoist boundary; the + # bubble reaches the root and Walker#call must append it (not drop it). + custom = Markbridge::Normalizer.default + custom.rule( + parent: Markbridge::AST::Document, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + img = image + tree = doc(img, text("b")) + custom.normalize(tree) + + # image removed from the front, appended at the end by the root fallback + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Text, Markbridge::AST::Image]) + expect(tree.children.last).to be(img) + end + end +end diff --git a/spec/unit/markbridge/normalizer_spec.rb b/spec/unit/markbridge/normalizer_spec.rb new file mode 100644 index 00000000..f6131ee7 --- /dev/null +++ b/spec/unit/markbridge/normalizer_spec.rb @@ -0,0 +1,398 @@ +# frozen_string_literal: true + +RSpec.describe Markbridge::Normalizer do + # Concise tree builders. `el` wraps a klass with children; the helpers + # name the common nodes. + def el(klass, *children, **kwargs) + node = kwargs.empty? ? klass.new : klass.new(**kwargs) + children.each { |child| node << child } + node + end + + def doc(*children) = el(Markbridge::AST::Document, *children) + def text(string) = Markbridge::AST::Text.new(string) + def url(*children, href: "https://ex.com") = el(Markbridge::AST::Url, *children, href:) + def image(src: "https://ex.com/i.png") = Markbridge::AST::Image.new(src:) + + # Any valid instance of the class; the rules only look at the class. + def instance(klass) + case klass.name.split("::").last + when "Heading" + klass.new(level: 1) + when "Align" + klass.new(alignment: "center") + when "Url" + klass.new(href: "u") + when "Image" + klass.new(src: "s") + when "Poll" + klass.new(name: "p") + when "Event" + klass.new(name: "e", starts_at: "2026-01-01") + else + klass.new + end + end + + # An instance of +container_klass+ holding +child+. + def wrap(container_klass, child) = instance(container_klass).tap { |c| c << child } + + def short(klass) = klass.name.split("::").last + + # The default rules do not move an image out of a link (that is Discourse + # policy a consumer adds). The examples below add that rule so they have a + # simple case to work with. + subject(:normalizer) do + described_class.default.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :hoist_after, + ) + end + + describe ".default" do + it "builds a fresh, customizable instance" do + one = described_class.default + two = described_class.default + expect(one).to be_a(described_class) + expect(one).not_to be(two) + expect(one).not_to be_frozen + end + + it "carries the default rules (a link inside a link is unwrapped)" do + tree = doc(url(url(text("x"), href: "b"), href: "a")) + described_class.default.normalize(tree) + + expect(tree.children.size).to eq(1) + expect(tree.children.first.href).to eq("a") + end + end + + describe ".shared_default" do + it "returns the same frozen instance on every call" do + expect(described_class.shared_default).to be_a(described_class) + expect(described_class.shared_default).to be(described_class.shared_default) + expect(described_class.shared_default).to be_frozen + end + + it "normalizes and can be reused, even though it is frozen" do + shared = described_class.shared_default + 2.times do + tree = doc(url(url(text("x"), href: "b"), href: "a")) + expect(shared.normalize(tree)).to eq( + [{ parent: "Url", child: "Url", strategy: :unwrap, count: 1 }], + ) + end + end + end + + describe "default rules" do + it "unwraps a link inside a link" do + expect(described_class.default.violations(doc(url(url(text("x")))))).to contain_exactly( + { parent: "Url", child: "Url", strategy: :unwrap }, + ) + end + + it "flags every block node inside every inline container" do + described_class::INLINE_CONTAINERS.each do |container| + described_class::BLOCK_NODES.each do |block| + next if container == block + + found = described_class.default.violations(doc(wrap(container, instance(block)))) + expect(found).to include( + { parent: short(container), child: short(block), strategy: :hoist_after }, + ), + "(#{container}, #{block}) not flagged" + end + end + end + + it "keeps an inline code span but flags a fenced one, in any inline container" do + described_class::INLINE_CONTAINERS.each do |container| + inline = el(Markbridge::AST::Code, text("x")) + fenced = el(Markbridge::AST::Code, text("a\nb")) + + expect(described_class.default.violations(doc(wrap(container, inline)))).to eq([]), + "inline code in #{container}" + expect(described_class.default.violations(doc(wrap(container, fenced)))).to contain_exactly( + { parent: short(container), child: "Code", strategy: :hoist_after }, + ), + "fenced code in #{container}" + end + end + + it "does not flag a node that is both an inline container and a block against itself" do + # Heading is in INLINE_CONTAINERS and in BLOCK_NODES. build_rules must not + # add a (Heading, Heading) hoist rule, or a heading in a heading would move. + heading = Markbridge::AST::Heading + tree = doc(wrap(heading, instance(heading))) + + expect(described_class.default.violations(tree)).to eq([]) + end + + it "does not add Discourse policy: an image or mention in a link is not flagged" do + expect(described_class.default.violations(doc(url(image)))).to eq([]) + mention = Markbridge::AST::Mention.new(name: "a") + expect(described_class.default.violations(doc(url(mention)))).to eq([]) + end + end + + describe "#rule" do + it "is chainable" do + expect( + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Mention, + strategy: :textify, + ), + ).to be(normalizer) + end + + it "replaces an existing rule for the same (parent, child) pair" do + tree = doc(url(image)) + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :drop) + normalizer.normalize(tree) + + # was :hoist_after, now :drop — image gone, nothing hoisted + expect(tree.children).to eq([tree.children.first]) + expect(tree.children.first.children).to eq([]) + end + + it "raises on a frozen (shared) instance" do + expect { + described_class.shared_default.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: :drop, + ) + }.to raise_error(FrozenError) + end + end + + describe "#normalize strategies" do + it "hoists an image out of a link, leaves the (empty) link, and reports it" do + tree = doc(url(image)) + report = normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Image]) + expect(tree.children.first.children).to eq([]) # empty Url survives (bare link) + expect(report).to contain_exactly( + { parent: "Url", child: "Image", strategy: :hoist_after, count: 1 }, + ) + end + + it "hoists to the OUTERMOST offending ancestor, across nested formatting" do + # image inside bold inside link → hoist after the LINK, not the bold + tree = doc(url(el(Markbridge::AST::Bold, image))) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Image]) + # bold emptied by the hoist → removed (no empty ** **) + expect(tree.children.first.children).to eq([]) + end + + it "prunes an emptied formatting wrapper but never an emptied Url" do + tree = doc(url(el(Markbridge::AST::Bold, image))) + normalizer.normalize(tree) + + expect(tree.descendants(Markbridge::AST::Bold)).to be_empty + expect(tree.children.first).to be_a(Markbridge::AST::Url) # kept as bare link + end + + it "unwraps an inner link, keeping its label text" do + inner = url(text("click"), href: "https://b.com") + tree = doc(url(inner, href: "https://a.com")) + report = normalizer.normalize(tree) + + expect(tree.children.size).to eq(1) + outer = tree.children.first + expect(outer).to be_a(Markbridge::AST::Url) + expect(outer.href).to eq("https://a.com") + expect(outer.children.map { |c| [c.class, c.respond_to?(:text) ? c.text : nil] }).to eq( + [[Markbridge::AST::Text, "click"]], + ) + expect(report).to contain_exactly( + { parent: "Url", child: "Url", strategy: :unwrap, count: 1 }, + ) + end + + it "textifies a node via a custom rule, projecting its plain text" do + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Mention, + strategy: :textify, + ) + tree = doc(url(text("hi "), Markbridge::AST::Mention.new(name: "alice"))) + normalizer.normalize(tree) + + link = tree.children.first + expect(link.children.size).to eq(1) # "hi " + "@alice" coalesced + expect(link.children.first.text).to eq("hi @alice") + end + + it "drops a node via a custom rule" do + normalizer.rule(parent: Markbridge::AST::Url, child: Markbridge::AST::Image, strategy: :drop) + tree = doc(url(text("label"), image)) + normalizer.normalize(tree) + + link = tree.children.first + expect(link.children.map(&:class)).to eq([Markbridge::AST::Text]) + end + + it "keeps a mention in a link silently (no report row)" do + tree = doc(url(Markbridge::AST::Mention.new(name: "alice"))) + report = normalizer.normalize(tree) + + expect(tree.children.first.children.map(&:class)).to eq([Markbridge::AST::Mention]) + expect(report).to be_empty + end + end + + describe "#normalize ordering (moved subtrees walk against their new-place stack)" do + it "keeps a legally-nested quote-in-quote intact when the outer quote is hoisted" do + inner_quote = el(Markbridge::AST::Quote, text("deep")) + outer_quote = el(Markbridge::AST::Quote, inner_quote) + tree = doc(url(outer_quote)) + normalizer.normalize(tree) + + # outer quote hoisted after the (empty) link; inner quote STILL nested + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Quote]) + hoisted = tree.children.last + expect(hoisted.children).to contain_exactly(be_a(Markbridge::AST::Quote)) + expect(hoisted.children.first.children.first.text).to eq("deep") + end + + it "leaves an image inside a quote inside a link where it is (in the quote)" do + quote = el(Markbridge::AST::Quote, image) + tree = doc(url(quote)) + normalizer.normalize(tree) + + # quote hoists out of the link; image stays IN the quote (not ripped out) + hoisted_quote = tree.children.last + expect(hoisted_quote).to be_a(Markbridge::AST::Quote) + expect(hoisted_quote.children).to contain_exactly(be_a(Markbridge::AST::Image)) + end + + it "preserves document order for multiple hoists to one link" do + img1 = Markbridge::AST::Image.new(src: "one") + img2 = Markbridge::AST::Image.new(src: "two") + tree = doc(url(img1, text("mid"), img2)) + normalizer.normalize(tree) + + # link keeps "mid"; images land after the link in original order + link, first, second = tree.children + expect(link).to be_a(Markbridge::AST::Url) + expect(link.children.map(&:text)).to eq(["mid"]) + expect([first.src, second.src]).to eq(%w[one two]) + end + end + + describe "#normalize fixpoint" do + it "reaches a fixpoint: a second normalize reports nothing" do + trees = [ + doc(url(el(Markbridge::AST::Bold, image))), + doc(url(url(text("x"), href: "b"), href: "a")), + doc(url(el(Markbridge::AST::Quote, el(Markbridge::AST::Quote, image)))), + doc(url(url(url(text("deep"), href: "c"), href: "b"), href: "a")), + ] + + trees.each do |tree| + first = normalizer.normalize(tree) + second = normalizer.normalize(tree) + expect(first).not_to be_empty + expect(second).to eq([]), "expected empty second report, got #{second.inspect}" + end + end + end + + describe "#normalize inline-code predicate (Url, Code)" do + it "keeps an inline code span in a link label" do + code = el(Markbridge::AST::Code, text("x")) + tree = doc(url(code)) + normalizer.normalize(tree) + + expect(tree.children.first).to be_a(Markbridge::AST::Url) + expect(tree.children.first.children).to contain_exactly(be_a(Markbridge::AST::Code)) + end + + it "hoists a multi-line (fenced) code block out of a link label" do + code = el(Markbridge::AST::Code, text("line1\nline2")) + tree = doc(url(code)) + normalizer.normalize(tree) + + expect(tree.children.map(&:class)).to eq([Markbridge::AST::Url, Markbridge::AST::Code]) + end + end + + describe "#normalize early exit" do + it "returns an empty report and leaves a clean tree untouched" do + tree = doc(el(Markbridge::AST::Bold, text("hi"))) + before = tree.children.first + report = normalizer.normalize(tree) + + expect(report).to eq([]) + expect(tree.children.first).to be(before) + end + end + + describe "#violations" do + it "reports would-be violations without mutating the tree" do + tree = doc(url(image)) + found = normalizer.violations(tree) + + expect(found).to contain_exactly({ parent: "Url", child: "Image", strategy: :hoist_after }) + # unchanged + expect(tree.children.first).to be_a(Markbridge::AST::Url) + expect(tree.children.first.children).to contain_exactly(be_a(Markbridge::AST::Image)) + end + + it "omits explicitly-kept nodes (a mention in a link)" do + tree = doc(url(Markbridge::AST::Mention.new(name: "alice"))) + expect(normalizer.violations(tree)).to eq([]) + end + + it "recurses into nested elements, finding violations at every depth" do + # image nested two elements deep inside a link, plus the quote itself + tree = doc(url(el(Markbridge::AST::Quote, el(Markbridge::AST::Bold, image)))) + found = normalizer.violations(tree) + + expect(found).to contain_exactly( + { parent: "Url", child: "Quote", strategy: :hoist_after }, + { parent: "Url", child: "Image", strategy: :hoist_after }, + ) + end + + it "passes the boundary to a callable rule" do + seen = [] + normalizer.rule( + parent: Markbridge::AST::Url, + child: Markbridge::AST::Image, + strategy: + lambda do |boundary, _node| + seen << boundary + :drop + end, + ) + link = url(image) + normalizer.violations(doc(link)) + + expect(seen).to eq([link]) # the offending Url, not nil + end + + it "resolves callable rules — inline code is kept (omitted), a code block reported" do + inline = doc(url(el(Markbridge::AST::Code, text("x")))) + block = doc(url(el(Markbridge::AST::Code, text("a\nb")))) + + expect(normalizer.violations(inline)).to eq([]) + expect(normalizer.violations(block)).to contain_exactly( + { parent: "Url", child: "Code", strategy: :hoist_after }, + ) + end + + it "is empty once the tree has been normalized (validation property)" do + tree = doc(url(el(Markbridge::AST::Bold, image))) + normalizer.normalize(tree) + expect(described_class.default.violations(tree)).to eq([]) + end + end +end diff --git a/spec/unit/markbridge/renderers/discourse/tags/event_tag_spec.rb b/spec/unit/markbridge/renderers/discourse/tags/event_tag_spec.rb index 69bc5d76..433f12d7 100644 --- a/spec/unit/markbridge/renderers/discourse/tags/event_tag_spec.rb +++ b/spec/unit/markbridge/renderers/discourse/tags/event_tag_spec.rb @@ -15,14 +15,14 @@ raw: "[event]ORIGINAL[/event]", ) - expect(tag.render(element, interface)).to eq("[event]ORIGINAL[/event]\n\n") + expect(tag.render(element, interface)).to eq("\n\n[event]ORIGINAL[/event]\n\n") end it "reconstructs BBCode with name and start when raw is missing" do element = Markbridge::AST::Event.new(name: "Meeting", starts_at: "2026-01-01 14:00") expect(tag.render(element, interface)).to eq( - %([event name="Meeting" start="2026-01-01 14:00"]\n[/event]\n\n), + %(\n\n[event name="Meeting" start="2026-01-01 14:00"]\n[/event]\n\n), ) end @@ -31,7 +31,7 @@ Markbridge::AST::Event.new(name: "Meeting", starts_at: "2026-01-01", ends_at: "2026-01-02") expect(tag.render(element, interface)).to eq( - %([event name="Meeting" start="2026-01-01" end="2026-01-02"]\n[/event]\n\n), + %(\n\n[event name="Meeting" start="2026-01-01" end="2026-01-02"]\n[/event]\n\n), ) end @@ -40,7 +40,7 @@ Markbridge::AST::Event.new(name: "Meeting", starts_at: "2026-01-01", status: "public") expect(tag.render(element, interface)).to eq( - %([event name="Meeting" start="2026-01-01" status="public"]\n[/event]\n\n), + %(\n\n[event name="Meeting" start="2026-01-01" status="public"]\n[/event]\n\n), ) end @@ -53,7 +53,7 @@ ) expect(tag.render(element, interface)).to eq( - %([event name="Meeting" start="2026-01-01" timezone="Europe/Vienna"]\n[/event]\n\n), + %(\n\n[event name="Meeting" start="2026-01-01" timezone="Europe/Vienna"]\n[/event]\n\n), ) end @@ -67,13 +67,14 @@ expect(result).not_to include("timezone=") end - it "emits a trailing blank line after a reconstructed event" do + it "brackets a reconstructed event with leading and trailing blank lines" do element = Markbridge::AST::Event.new(name: "Meeting", starts_at: "2026-04-24 10:00") + expect(tag.render(element, interface)).to start_with("\n\n[event") expect(tag.render(element, interface)).to end_with("[/event]\n\n") end - it "emits a trailing blank line after a raw-passthrough event" do + it "brackets a raw-passthrough event the same way" do element = Markbridge::AST::Event.new( name: "Meeting", @@ -81,26 +82,22 @@ raw: %([event name="Meeting" start="2026-04-24 10:00"]\n[/event]), ) + expect(tag.render(element, interface)).to start_with("\n\n[event") expect(tag.render(element, interface)).to end_with("[/event]\n\n") end - context "in html_mode" do - let(:context) { Markbridge::Renderers::Discourse::RenderContext.new([], html_mode: true) } - - it "wraps the BBCode in leading + trailing blank lines so CommonMark re-enters Markdown parsing" do - element = Markbridge::AST::Event.new(name: "Meeting", starts_at: "2026-01-01") - - expect(tag.render(element, interface)).to eq( - %(\n\n[event name="Meeting" start="2026-01-01"]\n[/event]\n\n), - ) - end - - it "wraps a raw-passthrough event in blank lines too" do - element = - Markbridge::AST::Event.new(name: "x", starts_at: "y", raw: "[event]ORIGINAL[/event]") - - expect(tag.render(element, interface)).to eq("\n\n[event]ORIGINAL[/event]\n\n") - end + # The stub is mode-agnostic: the same blank-line-bracketed island serves + # both a standalone block in Markdown and the html_mode contract (which + # is enforced by html_mode_contract_spec). + it "emits the same island form in html_mode" do + html_context = Markbridge::Renderers::Discourse::RenderContext.new([], html_mode: true) + html_interface = + Markbridge::Renderers::Discourse::RenderingInterface.new(renderer, html_context) + element = Markbridge::AST::Event.new(name: "Meeting", starts_at: "2026-01-01") + + expect(tag.render(element, html_interface)).to eq( + %(\n\n[event name="Meeting" start="2026-01-01"]\n[/event]\n\n), + ) end end end diff --git a/spec/unit/markbridge/renderers/discourse/tags/poll_tag_spec.rb b/spec/unit/markbridge/renderers/discourse/tags/poll_tag_spec.rb index cb31e45f..51959014 100644 --- a/spec/unit/markbridge/renderers/discourse/tags/poll_tag_spec.rb +++ b/spec/unit/markbridge/renderers/discourse/tags/poll_tag_spec.rb @@ -7,16 +7,16 @@ let(:interface) { Markbridge::Renderers::Discourse::RenderingInterface.new(renderer, context) } describe "#render" do - it "returns the raw BBCode verbatim when present" do + it "returns the raw BBCode verbatim when present, as a standalone block" do element = Markbridge::AST::Poll.new(raw: "[poll]ORIGINAL[/poll]") - expect(tag.render(element, interface)).to eq("[poll]ORIGINAL[/poll]\n\n") + expect(tag.render(element, interface)).to eq("\n\n[poll]ORIGINAL[/poll]\n\n") end it "reconstructs BBCode with options when raw is missing" do element = Markbridge::AST::Poll.new(options: %w[A B]) - expect(tag.render(element, interface)).to eq("[poll]\n* A\n* B\n[/poll]\n\n") + expect(tag.render(element, interface)).to eq("\n\n[poll]\n* A\n* B\n[/poll]\n\n") end it "omits the name attribute when it equals the default 'poll'" do @@ -70,35 +70,32 @@ it "produces an empty options block when there are no options" do element = Markbridge::AST::Poll.new - expect(tag.render(element, interface)).to eq("[poll]\n\n[/poll]\n\n") + expect(tag.render(element, interface)).to eq("\n\n[poll]\n\n[/poll]\n\n") end - it "emits a trailing blank line after a reconstructed poll" do + it "brackets a reconstructed poll with leading and trailing blank lines" do element = Markbridge::AST::Poll.new(name: "fav", type: "regular", options: %w[A B]) + expect(tag.render(element, interface)).to start_with("\n\n[poll") expect(tag.render(element, interface)).to end_with("[/poll]\n\n") end - it "emits a trailing blank line after a raw-passthrough poll" do + it "brackets a raw-passthrough poll the same way" do element = Markbridge::AST::Poll.new(raw: "[poll]\n* A\n* B\n[/poll]") - expect(tag.render(element, interface)).to eq("[poll]\n* A\n* B\n[/poll]\n\n") + expect(tag.render(element, interface)).to eq("\n\n[poll]\n* A\n* B\n[/poll]\n\n") end - context "in html_mode" do - let(:context) { Markbridge::Renderers::Discourse::RenderContext.new([], html_mode: true) } - - it "wraps the BBCode in leading + trailing blank lines so CommonMark re-enters Markdown parsing" do - element = Markbridge::AST::Poll.new(options: %w[A B]) - - expect(tag.render(element, interface)).to eq("\n\n[poll]\n* A\n* B\n[/poll]\n\n") - end - - it "wraps a raw-passthrough poll in blank lines too" do - element = Markbridge::AST::Poll.new(raw: "[poll]ORIGINAL[/poll]") + # The stub is mode-agnostic: the same blank-line-bracketed island serves + # both a standalone block in Markdown and the html_mode contract (which + # is enforced by html_mode_contract_spec). + it "emits the same island form in html_mode" do + html_context = Markbridge::Renderers::Discourse::RenderContext.new([], html_mode: true) + html_interface = + Markbridge::Renderers::Discourse::RenderingInterface.new(renderer, html_context) + element = Markbridge::AST::Poll.new(options: %w[A B]) - expect(tag.render(element, interface)).to eq("\n\n[poll]ORIGINAL[/poll]\n\n") - end + expect(tag.render(element, html_interface)).to eq("\n\n[poll]\n* A\n* B\n[/poll]\n\n") end end end