Fix false positive for type/str comparison warning (#5957)

Co-authored-by: Laurenz <laurmaedje@gmail.com>

crates/typst-cli/src/watch.rs

@@ -204,6 +204,10 @@ impl Watcher {
                 let event = event
                     .map_err(|err| eco_format!("failed to watch dependencies ({err})"))?;
 
+                if !is_relevant_event_kind(&event.kind) {
+                    continue;
+                }
+
                 // Workaround for notify-rs' implicit unwatch on remove/rename
                 // (triggered by some editors when saving files) with the
                 // inotify backend. By keeping track of the potentially
@@ -224,7 +228,17 @@ impl Watcher {
                     }
                 }
 
-                relevant |= self.is_event_relevant(&event);
+                // Don't recompile because the output file changed.
+                // FIXME: This doesn't work properly for multifile image export.
+                if event
+                    .paths
+                    .iter()
+                    .all(|path| is_same_file(path, &self.output).unwrap_or(false))
+                {
+                    continue;
+                }
+
+                relevant = true;
             }
 
             // If we found a relevant event or if any of the missing files now
@@ -234,32 +248,23 @@ impl Watcher {
             }
         }
     }
+}
 
-    /// Whether a watch event is relevant for compilation.
+/// Whether a kind of watch event is relevant for compilation.
-    fn is_event_relevant(&self, event: &notify::Event) -> bool {
+fn is_relevant_event_kind(kind: &notify::EventKind) -> bool {
-        // Never recompile because the output file changed.
+    match kind {
-        if event
+        notify::EventKind::Any => true,
-            .paths
+        notify::EventKind::Access(_) => false,
-            .iter()
+        notify::EventKind::Create(_) => true,
-            .all(|path| is_same_file(path, &self.output).unwrap_or(false))
+        notify::EventKind::Modify(kind) => match kind {
-        {
+            notify::event::ModifyKind::Any => true,
-            return false;
+            notify::event::ModifyKind::Data(_) => true,
-        }
+            notify::event::ModifyKind::Metadata(_) => false,
-
+            notify::event::ModifyKind::Name(_) => true,
-        match &event.kind {
+            notify::event::ModifyKind::Other => false,
-            notify::EventKind::Any => true,
+        },
-            notify::EventKind::Access(_) => false,
+        notify::EventKind::Remove(_) => true,
-            notify::EventKind::Create(_) => true,
+        notify::EventKind::Other => false,
-            notify::EventKind::Modify(kind) => match kind {
-                notify::event::ModifyKind::Any => true,
-                notify::event::ModifyKind::Data(_) => true,
-                notify::event::ModifyKind::Metadata(_) => false,
-                notify::event::ModifyKind::Name(_) => true,
-                notify::event::ModifyKind::Other => false,
-            },
-            notify::EventKind::Remove(_) => true,
-            notify::EventKind::Other => false,
-        }
     }
 }
 

crates/typst-html/src/lib.rs

@@ -83,8 +83,8 @@ fn html_document_impl(
     )?;
 
     let output = handle_list(&mut engine, &mut locator, children.iter().copied())?;
+    let introspector = Introspector::html(&output);
     let root = root_element(output, &info)?;
-    let introspector = Introspector::html(&root);
 
     Ok(HtmlDocument { info, root, introspector })
 }
@@ -307,18 +307,18 @@ fn head_element(info: &DocumentInfo) -> HtmlElement {
 
 /// Determine which kind of output the user generated.
 fn classify_output(mut output: Vec<HtmlNode>) -> SourceResult<OutputKind> {
-    let len = output.len();
+    let count = output.iter().filter(|node| !matches!(node, HtmlNode::Tag(_))).count();
     for node in &mut output {
         let HtmlNode::Element(elem) = node else { continue };
         let tag = elem.tag;
         let mut take = || std::mem::replace(elem, HtmlElement::new(tag::html));
-        match (tag, len) {
+        match (tag, count) {
             (tag::html, 1) => return Ok(OutputKind::Html(take())),
             (tag::body, 1) => return Ok(OutputKind::Body(take())),
             (tag::html | tag::body, _) => bail!(
                 elem.span,
                 "`{}` element must be the only element in the document",
-                elem.tag
+                elem.tag,
             ),
             _ => {}
         }

crates/typst-library/src/foundations/func.rs

@@ -437,10 +437,10 @@ impl PartialEq for Func {
     }
 }
 
-impl PartialEq<&NativeFuncData> for Func {
+impl PartialEq<&'static NativeFuncData> for Func {
-    fn eq(&self, other: &&NativeFuncData) -> bool {
+    fn eq(&self, other: &&'static NativeFuncData) -> bool {
         match &self.repr {
-            Repr::Native(native) => native.function == other.function,
+            Repr::Native(native) => *native == Static(*other),
             _ => false,
         }
     }

crates/typst-library/src/foundations/ops.rs

@@ -498,7 +498,7 @@ pub fn equal(lhs: &Value, rhs: &Value, sink: &mut dyn DeprecationSink) -> bool {
 
         // Type compatibility.
         (Type(ty), Str(str)) | (Str(str), Type(ty)) => {
-            warn_type_str_equal(sink);
+            warn_type_str_equal(sink, str);
             ty.compat_name() == str.as_str()
         }
 
@@ -647,14 +647,17 @@ fn warn_type_str_join(sink: &mut dyn DeprecationSink) {
 }
 
 #[cold]
-fn warn_type_str_equal(sink: &mut dyn DeprecationSink) {
+fn warn_type_str_equal(sink: &mut dyn DeprecationSink, s: &str) {
-    sink.emit_with_hints(
+    // Only warn if `s` looks like a type name to prevent false positives.
-        "comparing strings with types is deprecated",
+    if is_compat_type_name(s) {
-        &[
+        sink.emit_with_hints(
-            "compare with the literal type instead",
+            "comparing strings with types is deprecated",
-            "this comparison will always return `false` in future Typst releases",
+            &[
-        ],
+                "compare with the literal type instead",
-    );
+                "this comparison will always return `false` in future Typst releases",
+            ],
+        );
+    }
 }
 
 #[cold]
@@ -672,3 +675,44 @@ fn warn_type_in_dict(sink: &mut dyn DeprecationSink) {
         &["this compatibility behavior only exists because `type` used to return a string"],
     );
 }
+
+fn is_compat_type_name(s: &str) -> bool {
+    matches!(
+        s,
+        "boolean"
+            | "alignment"
+            | "angle"
+            | "arguments"
+            | "array"
+            | "bytes"
+            | "color"
+            | "content"
+            | "counter"
+            | "datetime"
+            | "decimal"
+            | "dictionary"
+            | "direction"
+            | "duration"
+            | "float"
+            | "fraction"
+            | "function"
+            | "gradient"
+            | "integer"
+            | "label"
+            | "length"
+            | "location"
+            | "module"
+            | "pattern"
+            | "ratio"
+            | "regex"
+            | "relative length"
+            | "selector"
+            | "state"
+            | "string"
+            | "stroke"
+            | "symbol"
+            | "tiling"
+            | "type"
+            | "version"
+    )
+}

crates/typst-library/src/introspection/introspector.rs

@@ -10,7 +10,7 @@ use typst_utils::NonZeroExt;
 
 use crate::diag::{bail, StrResult};
 use crate::foundations::{Content, Label, Repr, Selector};
-use crate::html::{HtmlElement, HtmlNode};
+use crate::html::HtmlNode;
 use crate::introspection::{Location, Tag};
 use crate::layout::{Frame, FrameItem, Page, Point, Position, Transform};
 use crate::model::Numbering;
@@ -55,8 +55,8 @@ impl Introspector {
 
     /// Creates an introspector for HTML.
     #[typst_macros::time(name = "introspect html")]
-    pub fn html(root: &HtmlElement) -> Self {
+    pub fn html(output: &[HtmlNode]) -> Self {
-        IntrospectorBuilder::new().build_html(root)
+        IntrospectorBuilder::new().build_html(output)
     }
 
     /// Iterates over all locatable elements.
@@ -392,9 +392,9 @@ impl IntrospectorBuilder {
     }
 
     /// Build an introspector for an HTML document.
-    fn build_html(mut self, root: &HtmlElement) -> Introspector {
+    fn build_html(mut self, output: &[HtmlNode]) -> Introspector {
         let mut elems = Vec::new();
-        self.discover_in_html(&mut elems, root);
+        self.discover_in_html(&mut elems, output);
         self.finalize(elems)
     }
 
@@ -434,16 +434,16 @@ impl IntrospectorBuilder {
     }
 
     /// Processes the tags in the HTML element.
-    fn discover_in_html(&mut self, sink: &mut Vec<Pair>, elem: &HtmlElement) {
+    fn discover_in_html(&mut self, sink: &mut Vec<Pair>, nodes: &[HtmlNode]) {
-        for child in &elem.children {
+        for node in nodes {
-            match child {
+            match node {
                 HtmlNode::Tag(tag) => self.discover_in_tag(
                     sink,
                     tag,
                     Position { page: NonZeroUsize::ONE, point: Point::zero() },
                 ),
                 HtmlNode::Text(_, _) => {}
-                HtmlNode::Element(elem) => self.discover_in_html(sink, elem),
+                HtmlNode::Element(elem) => self.discover_in_html(sink, &elem.children),
                 HtmlNode::Frame(frame) => self.discover_in_frame(
                     sink,
                     frame,

crates/typst-library/src/layout/grid/resolve.rs

@@ -1526,11 +1526,7 @@ impl<'a> CellGrid<'a> {
         self.entry(x, y).map(|entry| match entry {
             Entry::Cell(_) => Axes::new(x, y),
             Entry::Merged { parent } => {
-                let c = if self.has_gutter {
+                let c = self.non_gutter_column_count();
-                    1 + self.cols.len() / 2
-                } else {
-                    self.cols.len()
-                };
                 let factor = if self.has_gutter { 2 } else { 1 };
                 Axes::new(factor * (*parent % c), factor * (*parent / c))
             }
@@ -1602,6 +1598,21 @@ impl<'a> CellGrid<'a> {
             cell.rowspan.get()
         }
     }
+
+    #[inline]
+    pub fn non_gutter_column_count(&self) -> usize {
+        if self.has_gutter {
+            // Calculation: With gutters, we have
+            // 'cols = 2 * (non-gutter cols) - 1', since there is a gutter
+            // column between each regular column. Therefore,
+            // 'floor(cols / 2)' will be equal to
+            // 'floor(non-gutter cols - 1/2) = non-gutter-cols - 1',
+            // so 'non-gutter cols = 1 + floor(cols / 2)'.
+            1 + self.cols.len() / 2
+        } else {
+            self.cols.len()
+        }
+    }
 }
 
 /// Given a cell's requested x and y, the vector with the resolved cell

crates/typst-library/src/model/table.rs

@@ -282,7 +282,7 @@ fn show_cell_html(tag: HtmlTag, cell: &Cell, styles: StyleChain) -> Content {
 
 fn show_cellgrid_html(grid: CellGrid, styles: StyleChain) -> Content {
     let elem = |tag, body| HtmlElem::new(tag).with_body(Some(body)).pack();
-    let mut rows: Vec<_> = grid.entries.chunks(grid.cols.len()).collect();
+    let mut rows: Vec<_> = grid.entries.chunks(grid.non_gutter_column_count()).collect();
 
     let tr = |tag, row: &[Entry]| {
         let row = row

tests/ref/html/col-gutter-table.html

@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+  </head>
+  <body>
+    <table>
+      <tr>
+        <td>a</td>
+        <td>b</td>
+        <td>c</td>
+      </tr>
+      <tr>
+        <td>d</td>
+        <td>e</td>
+        <td>f</td>
+      </tr>
+      <tr>
+        <td>g</td>
+        <td>h</td>
+        <td>i</td>
+      </tr>
+    </table>
+  </body>
+</html>

tests/ref/html/col-row-gutter-table.html

@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+  </head>
+  <body>
+    <table>
+      <tr>
+        <td>a</td>
+        <td>b</td>
+        <td>c</td>
+      </tr>
+      <tr>
+        <td>d</td>
+        <td>e</td>
+        <td>f</td>
+      </tr>
+      <tr>
+        <td>g</td>
+        <td>h</td>
+        <td>i</td>
+      </tr>
+    </table>
+  </body>
+</html>

tests/ref/html/html-elem-alone-context.html

@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<html></html>

tests/ref/html/html-elem-metadata.html

@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<html>Hi</html>

tests/ref/html/row-gutter-table.html

@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1">
+  </head>
+  <body>
+    <table>
+      <tr>
+        <td>a</td>
+        <td>b</td>
+        <td>c</td>
+      </tr>
+      <tr>
+        <td>d</td>
+        <td>e</td>
+        <td>f</td>
+      </tr>
+      <tr>
+        <td>g</td>
+        <td>h</td>
+        <td>i</td>
+      </tr>
+    </table>
+  </body>
+</html>

tests/suite/foundations/type.typ

@@ -30,6 +30,8 @@
 // Hint: 7-26 compare with the literal type instead
 // Hint: 7-26 this comparison will always return `false` in future Typst releases
 #test(type(10) != "float", true)
+// This is not a warning.
+#test(type(10) in ("any", str, int), true)
 
 --- type-string-compatibility-in-array ---
 // Warning: 7-35 comparing strings with types is deprecated

tests/suite/html/elem.typ

@@ -0,0 +1,15 @@
+--- html-elem-alone-context html ---
+#context html.elem("html")
+
+--- html-elem-not-alone html ---
+// Error: 2-19 `<html>` element must be the only element in the document
+#html.elem("html")
+Text
+
+--- html-elem-metadata html ---
+#html.elem("html", context {
+  let val = query(<l>).first().value
+  test(val, "Hi")
+  val
+})
+#metadata("Hi") <l>

tests/suite/layout/grid/html.typ

@@ -30,3 +30,30 @@
     [row],
   ),
 )
+
+--- col-gutter-table html ---
+#table(
+  columns: 3,
+  column-gutter: 3pt,
+  [a], [b], [c],
+  [d], [e], [f],
+  [g], [h], [i]
+)
+
+--- row-gutter-table html ---
+#table(
+  columns: 3,
+  row-gutter: 3pt,
+  [a], [b], [c],
+  [d], [e], [f],
+  [g], [h], [i]
+)
+
+--- col-row-gutter-table html ---
+#table(
+  columns: 3,
+  gutter: 3pt,
+  [a], [b], [c],
+  [d], [e], [f],
+  [g], [h], [i]
+)