mark various things pub(crate)

Change various items from `pub` to `pub(crate)`. These items weren't
actually public, since their modules weren't exported as pub. This
doesn't change that, but it makes it clearer in the code.

Also add a CI check (and a helper script) to ensure that I'll maintain
this practice going forward.

Resolves #383

Author: yshavit

.github/workflows/rust.yml

@@ -83,3 +83,35 @@ jobs:
       - uses: actions/checkout@v4
       - name: cargo fmt
         run: cargo fmt --check
+
+  vis-keywords:
+    needs: build
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: check file
+        run: file scripts/flatten-rustdoc-json
+
+      - name: cargo docs (public only)
+        run: cargo +nightly rustdoc --lib -- -Zunstable-options --output-format json
+
+      - name: flatten rustdoc json (public only)
+        run: scripts/flatten-rustdoc-json > target/items-public-only.txt
+
+      - name: cargo docs (all)
+        run: cargo +nightly rustdoc --lib -- -Zunstable-options --output-format json --document-private-items
+
+      - name: flatten rustdoc json (all)
+        run: IGNORE_ITEMS='enum mdq::query::pest::Rule' scripts/flatten-rustdoc-json > target/items-all.txt
+
+      - name: compare
+        run: |
+          if diff -y --suppress-common-lines target/items-public-only.txt target/items-all.txt; then
+            echo "::notice title=pub-vis::all pub items are actually public"
+          else
+            echo "::error title=pub-vis::some items marked pub are not actually public"
+            exit 1
+          fi

scripts/flatten-rustdoc-json

@@ -0,0 +1,50 @@
+#!/usr/bin/env python3
+
+import os
+import json
+import sys
+
+my_dir = os.path.dirname(os.path.realpath(__file__))
+repo_dir = os.path.abspath(os.path.join(my_dir, '..'))
+
+DOCS_JSON_PATH = 'target/doc/mdq.json'
+
+IGNORES = set([e for e in os.environ.get('IGNORE_ITEMS', '').split(',') if e])
+if IGNORES:
+    print(f'Ignoring: {IGNORES}', file=sys.stderr)
+
+with open(DOCS_JSON_PATH) as fh:
+    docs_json = json.load(fh)
+
+vis_by_item_id = {}
+path_by_item_id = {}
+kind_by_item_id = {}
+
+for item_id, item in docs_json["index"].items():
+    vis = item["visibility"] # this is the vis on the item itself, not the effective vis from mod.rs imports
+    if isinstance(vis, dict) and ('restricted' in vis):
+        vis = 'restricted'
+    vis_by_item_id[item_id] = vis
+
+for item_id, item in docs_json["paths"].items():
+    kind_by_item_id[item_id] = item["kind"]
+    path_by_item_id[item_id] = '::'.join(item["path"])
+
+lines = []
+for item_id, vis in vis_by_item_id.items():
+    if vis != 'public':
+        continue
+    item_kind = kind_by_item_id.get(item_id)
+    item_path = path_by_item_id.get(item_id)
+    if item_kind is None or item_path is None:
+        continue
+    line = f'{item_kind} {item_path}' 
+    if line in IGNORES:
+        continue
+    lines.append(line)
+
+lines.sort()
+
+for line in lines:
+    print(line)
+

src/md_elem/concatenate.rs

@@ -1,4 +1,4 @@
-pub trait Concatenate: Sized {
+pub(crate) trait Concatenate: Sized {
     fn try_concatenate(&mut self, other: Self) -> Result<(), Self>;
 
     fn concatenate_similar(items: Vec<Self>) -> Vec<Self> {

src/md_elem/tree.rs

@@ -215,9 +215,9 @@ pub(crate) struct ReadOptions {
     /// [1]: https://example.com/one
     /// [1]: https://example.com/one
     /// ```
-    pub validate_no_conflicting_links: bool,
+    pub(crate) validate_no_conflicting_links: bool,
 
-    pub allow_unknown_markdown: bool,
+    pub(crate) allow_unknown_markdown: bool,
 }
 
 /// Various error conditions that can come from trying to parse Markdown.
@@ -1803,14 +1803,14 @@ mod tests {
     use crate::util::utils_for_test::*;
 
     impl MdContext {
-        pub fn empty() -> Self {
+        pub(crate) fn empty() -> Self {
             Self {
                 footnotes: Default::default(),
                 empty_md_elems: vec![],
             }
         }
 
-        pub fn with<S: Into<FootnoteId>>(mut self, footnote_id: S, body: Vec<MdElem>) -> Self {
+        pub(crate) fn with<S: Into<FootnoteId>>(mut self, footnote_id: S, body: Vec<MdElem>) -> Self {
             self.footnotes.insert(footnote_id.into(), body);
             self
         }

src/output/fmt_md.rs

@@ -154,7 +154,7 @@ impl<'md> MdWriterState<'_, 'md> {
         }
     }
 
-    pub fn write_one_md<W>(&mut self, out: &mut Output<W>, node_ref: &'md MdElem)
+    pub(crate) fn write_one_md<W>(&mut self, out: &mut Output<W>, node_ref: &'md MdElem)
     where
         W: SimpleWrite,
     {
@@ -528,7 +528,7 @@ enum DefinitionsToWrite {
 }
 
 #[cfg(test)]
-pub mod tests {
+pub(crate) mod tests {
     use indoc::indoc;
 
     use super::*;

src/output/fmt_md_inlines.rs

@@ -26,8 +26,8 @@ pub(crate) struct MdInlinesWriter<'md> {
 }
 
 struct PendingReferences<'md> {
-    pub links: HashMap<LinkLabel<'md>, UrlAndTitle<'md>>,
-    pub footnotes: HashSet<&'md FootnoteId>,
+    links: HashMap<LinkLabel<'md>, UrlAndTitle<'md>>,
+    footnotes: HashSet<&'md FootnoteId>,
 }
 
 impl PendingReferences<'_> {
@@ -41,9 +41,9 @@ impl PendingReferences<'_> {
 
 #[derive(Serialize, Debug, PartialEq, Eq, Copy, Clone, Hash)]
 pub(crate) struct UrlAndTitle<'md> {
-    pub url: &'md String,
+    pub(crate) url: &'md String,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub title: &'md Option<String>,
+    pub(crate) title: &'md Option<String>,
 }
 
 #[derive(Debug, Copy, Clone)]
@@ -73,7 +73,7 @@ impl<'md> LinkLike<'md> for &'md Image {
 }
 
 impl<'md> MdInlinesWriter<'md> {
-    pub fn new(ctx: &'md MdContext, options: InlineElemOptions) -> Self {
+    pub(crate) fn new(ctx: &'md MdContext, options: InlineElemOptions) -> Self {
         let pending_refs_capacity = 8; // arbitrary guess
         Self {
             ctx,
@@ -85,27 +85,27 @@ impl<'md> MdInlinesWriter<'md> {
         }
     }
 
-    pub fn has_pending_links(&self) -> bool {
+    pub(crate) fn has_pending_links(&self) -> bool {
         !self.pending_references.links.is_empty()
     }
 
-    pub fn has_pending_footnotes(&self) -> bool {
+    pub(crate) fn has_pending_footnotes(&self) -> bool {
         !self.pending_references.footnotes.is_empty()
     }
 
-    pub fn count_pending_links(&self) -> usize {
+    pub(crate) fn count_pending_links(&self) -> usize {
         self.pending_references.links.len()
     }
 
-    pub fn count_pending_footnotes(&self) -> usize {
+    pub(crate) fn count_pending_footnotes(&self) -> usize {
         self.pending_references.footnotes.len()
     }
 
-    pub fn drain_pending_links(&mut self) -> Vec<(LinkLabel<'md>, UrlAndTitle<'md>)> {
+    pub(crate) fn drain_pending_links(&mut self) -> Vec<(LinkLabel<'md>, UrlAndTitle<'md>)> {
         self.pending_references.links.drain().collect()
     }
 
-    pub fn drain_pending_footnotes(&mut self) -> Vec<(String, &'md Vec<MdElem>)> {
+    pub(crate) fn drain_pending_footnotes(&mut self) -> Vec<(String, &'md Vec<MdElem>)> {
         let mut result = Vec::with_capacity(self.pending_references.footnotes.len());
         let mut to_stringer = self.footnote_transformer.new_to_stringer();
 
@@ -118,7 +118,7 @@ impl<'md> MdInlinesWriter<'md> {
         result
     }
 
-    pub fn write_line<I, W>(&mut self, out: &mut Output<W>, elems: I)
+    pub(crate) fn write_line<I, W>(&mut self, out: &mut Output<W>, elems: I)
     where
         I: IntoIterator<Item = &'md Inline>,
         W: SimpleWrite,
@@ -128,7 +128,7 @@ impl<'md> MdInlinesWriter<'md> {
         }
     }
 
-    pub fn write_inline_element<W>(&mut self, out: &mut Output<W>, elem: &'md Inline)
+    pub(crate) fn write_inline_element<W>(&mut self, out: &mut Output<W>, elem: &'md Inline)
     where
         W: SimpleWrite,
     {
@@ -269,7 +269,7 @@ impl<'md> MdInlinesWriter<'md> {
     ///
     /// The `contents` function is what writes e.g. `an inline link` above. It's a function because it may be a recursive
     /// call into [write_line] (for links) or just simple text (for image alts).
-    pub fn write_linklike<W, L>(&mut self, out: &mut Output<W>, link_like: L)
+    pub(crate) fn write_linklike<W, L>(&mut self, out: &mut Output<W>, link_like: L)
     where
         W: SimpleWrite,
         L: LinkLike<'md> + Copy,
@@ -372,7 +372,7 @@ impl<'md> MdInlinesWriter<'md> {
         });
     }
 
-    pub fn write_url_title<W>(&mut self, out: &mut Output<W>, title: &Option<String>)
+    pub(crate) fn write_url_title<W>(&mut self, out: &mut Output<W>, title: &Option<String>)
     where
         W: SimpleWrite,
     {
@@ -415,7 +415,7 @@ enum TitleQuote {
 }
 
 impl TitleQuote {
-    pub fn find_best_strategy(text: &str) -> Self {
+    pub(crate) fn find_best_strategy(text: &str) -> Self {
         [Self::Double, Self::Single, Self::Paren]
             .into_iter()
             .find(|strategy| !strategy.has_conflicts(text))

src/output/fmt_plain_writer.rs

@@ -1,7 +1,7 @@
 use std::cmp::min;
 use std::io::Write;
 
-pub struct NewlineCollapser<W> {
+pub(crate) struct NewlineCollapser<W> {
     max_newlines: usize,
     underlying: W,
     /// How many newlines are in this current stretch, or None if we haven't written anything yet.
@@ -12,22 +12,22 @@ impl<W> NewlineCollapser<W>
 where
     W: Write,
 {
-    pub fn new(underlying: W, max_newlines: usize) -> Self {
+    pub(crate) fn new(underlying: W, max_newlines: usize) -> Self {
         Self {
             max_newlines,
             underlying,
             current_newline_stretch: None,
         }
     }
 
-    pub fn have_pending_newlines(&self) -> bool {
+    pub(crate) fn have_pending_newlines(&self) -> bool {
         match self.current_newline_stretch {
             None | Some(0) => false,
             Some(_) => true,
         }
     }
 
-    pub fn take_underlying(self) -> W {
+    pub(crate) fn take_underlying(self) -> W {
         self.underlying
     }
 

src/output/footnote_transform.rs

@@ -1,23 +1,23 @@
 use crate::util::output::{Output, SimpleWrite};
 use std::collections::HashMap;
 
-pub struct FootnoteTransformer<'md> {
+pub(crate) struct FootnoteTransformer<'md> {
     mappings: Option<HashMap<&'md str, usize>>,
 }
 
-pub struct FootnoteTransformerToString<'a, 'md> {
+pub(crate) struct FootnoteTransformerToString<'a, 'md> {
     transformer: &'a mut FootnoteTransformer<'md>,
     scratch: Output<String>,
 }
 
 impl<'md> FootnoteTransformer<'md> {
-    pub fn new(active: bool) -> Self {
+    pub(crate) fn new(active: bool) -> Self {
         Self {
             mappings: if active { Some(HashMap::default()) } else { None },
         }
     }
 
-    pub fn write<W>(&mut self, out: &mut Output<W>, label: &'md str)
+    pub(crate) fn write<W>(&mut self, out: &mut Output<W>, label: &'md str)
     where
         W: SimpleWrite,
     {
@@ -31,13 +31,13 @@ impl<'md> FootnoteTransformer<'md> {
         }
     }
 
-    pub fn new_to_stringer<'a>(&'a mut self) -> FootnoteTransformerToString<'a, 'md> {
+    pub(crate) fn new_to_stringer<'a>(&'a mut self) -> FootnoteTransformerToString<'a, 'md> {
         FootnoteTransformerToString::new(self)
     }
 }
 
 impl<'a, 'md> FootnoteTransformerToString<'a, 'md> {
-    pub fn transform(&mut self, label: &'md str) -> String {
+    pub(crate) fn transform(&mut self, label: &'md str) -> String {
         let len = self.transformed_label_len(label);
         _ = self.scratch.replace_underlying(String::with_capacity(len)).unwrap();
         self.transformer.write(&mut self.scratch, label);

src/output/link_transform.rs

@@ -36,7 +36,7 @@ pub(crate) enum LinkLabel<'md> {
 }
 
 impl<'md> LinkLabel<'md> {
-    pub fn get_sort_string(&self, ctx: &'md MdContext) -> String {
+    pub(crate) fn get_sort_string(&self, ctx: &'md MdContext) -> String {
         // There may be a way to Cow this so that we don't have to copy the ::Text string, but I can't find it.
         match self {
             LinkLabel::Text(s) => s.to_string(),
@@ -124,7 +124,7 @@ impl<'md> LinkTransformation<'md> {
 }
 
 impl LinkTransformer {
-    pub fn transform_variant(&self) -> LinkTransform {
+    pub(crate) fn transform_variant(&self) -> LinkTransform {
         match self.delegate {
             LinkTransformState::Keep => LinkTransform::Keep,
             LinkTransformState::Inline => LinkTransform::Inline,

src/query/error.rs

@@ -97,9 +97,9 @@ impl From<crate::query::Error> for InnerParseError {
 
 /// Like a [pest::Span], but without a reference to the underlying `&str`, and thus cheaply Copyable.
 #[derive(Copy, Clone, Default, Debug, PartialEq, Eq, Hash)]
-pub struct DetachedSpan {
-    pub start: usize,
-    pub end: usize,
+pub(crate) struct DetachedSpan {
+    pub(crate) start: usize,
+    pub(crate) end: usize,
 }
 
 impl From<pest::Span<'_>> for DetachedSpan {