diff --git a/lib/compiler/Driver.ml b/lib/compiler/Driver.ml index 1e1121d..17d41d5 100644 --- a/lib/compiler/Driver.ml +++ b/lib/compiler/Driver.ml @@ -45,8 +45,9 @@ let update ?(bail = true) ?(progress = Phases.no_progress) (action : Action.t) | Some code -> let result, errors = Phases.expand forest code in forest.={uri} <- Tree result; + Diagnostics_store.replace forest.diagnostics uri Expand errors; (report ~errors ~and_then:(Eval uri), forest) - end + end | Render uri -> let errors = Phases.render uri in (report ~errors ~and_then:Done, forest) diff --git a/lib/compiler/Phases.ml b/lib/compiler/Phases.ml index 35d7e41..258d988 100644 --- a/lib/compiler/Phases.ml +++ b/lib/compiler/Phases.ml @@ -90,7 +90,7 @@ let parse (forest : State.t) uri = match Parse.parse_document tree with | Ok code -> forest.={uri} <- Tree code; - Diagnostics_store.set forest.diagnostics uri []; + Diagnostics_store.clear_phase forest.diagnostics uri Parse; Expand uri | Error parse_error -> let errors = [Error.parse_error parse_error] in @@ -98,11 +98,15 @@ let parse (forest : State.t) uri = report ~errors ~and_then:Done end ~none:begin match State.get_document ~forest uri with - | Some doc -> begin - match Parse.parse_document doc with - | Error e -> report ~errors:[Error.parse_error e] ~and_then:Done + | Some doc -> + begin match Parse.parse_document doc with + | Error e -> + let errors = [Error.parse_error e] in + Diagnostics_store.set forest.diagnostics uri errors; + report ~errors ~and_then:Done | Ok code -> forest.={uri} <- Tree code; + Diagnostics_store.clear_phase forest.diagnostics uri Parse; Expand uri end | None -> assert false @@ -119,7 +123,7 @@ let parse_all (forest : State.t) ~progress = match Parse.parse_document document with | Ok tree -> begin forest.={uri} <- Tree tree; - Diagnostics_store.clear forest.diagnostics uri; + Diagnostics_store.clear_phase forest.diagnostics uri Parse; None end | Error error -> @@ -135,8 +139,11 @@ let reparse (doc : Lsp.Text_document.t) (forest : State.t) = URI.of_lsp_uri ~base:forest.config.url @@ Lsp.Text_document.documentUri doc in begin match Parse.parse_document doc with - | Ok code -> forest.={uri} <- Tree code - | Error d -> Diagnostics_store.set forest.diagnostics uri [Error.parse_error d] + | Ok code -> + forest.={uri} <- Tree code; + Diagnostics_store.clear_phase forest.diagnostics uri Parse + | Error d -> + Diagnostics_store.set forest.diagnostics uri [Error.parse_error d] end let build_import_graph ~(forest : State.t) : Error.t list * Forest_graph.t = @@ -161,7 +168,7 @@ let expand_all (forest : State.t) ~progress = let expanded, errors = Expand.expand_tree ~forest tree in diagnostics := errors @ !diagnostics; forest.={uri} <- Tree expanded; - Diagnostics_store.set forest.diagnostics uri errors + Diagnostics_store.replace forest.diagnostics uri Expand errors | None -> forest.log.err ~src:Log.Src.compiler (fun m -> m "expanding: no source code for %a" URI.pp uri); @@ -194,20 +201,46 @@ let run_jobs (forest : State.t) jobs ~progress = result in begin - (* It is probably not save to plant the articles in parallel, so this is + (* It is probably not safe to plant the articles in parallel, so this is done sequentially! *) let@ result = List.iter @~ resources_to_plant in match result with | Ok resource -> State.plant_resource ~forest resource - | Error tex_error -> begin - match guess_uri (Error.tex_range tex_error) with + | Error tex_error -> + begin match guess_uri (Error.tex_range tex_error) with | None -> assert false | Some uri -> let uri = URI.of_lsp_uri ~base:forest.config.url uri in - Diagnostics_store.set forest.diagnostics uri [Error.of_tex_error tex_error] - end + Diagnostics_store.set forest.diagnostics uri + [Error.of_tex_error tex_error] + end end +let subtree_uris (articles : T.content T.article list) = + let@ article = List.filter_map @~ articles in + Option.bind article.frontmatter.designated_parent + (Fun.const article.frontmatter.uri) + +let flag_duplicate_subtrees ~(forest : State.t) ?owner + (articles : T.content T.article list) = + let subtrees = subtree_uris articles in + let counts = URI.Tbl.create 8 in + let count_uri uri = + let n = 1 + Option.value ~default:0 (URI.Tbl.find_opt counts uri) in + URI.Tbl.replace counts uri n + in + List.iter count_uri subtrees; + begin + let@ owner = Option.iter @~ owner in + State.set_owned_subtrees ~forest ~owner subtrees + end; + URI.Tbl.iter + (fun uri n -> + if n > 1 then + Diagnostics_store.set forest.diagnostics uri [Error.duplicate_tree ~uri] + else Diagnostics_store.clear_phase forest.diagnostics uri Duplicate) + counts + let organize results = let open List in Pair.map_fst (split >>> Pair.map concat concat) @@ -227,14 +260,20 @@ let eval (forest : State.t) ~progress = organize @@ let@ uri, tree = List.map @~ expanded in - let result = Eval.(eval_tree ~config ~uri tree) in + let value, errors = Eval.(eval_tree ~config ~uri tree) in + begin + let@ {articles; _} = Option.iter @~ value in + State.set_owned_subtrees ~forest ~owner:uri (subtree_uris articles) + end; tick (); - result + (value, errors) in + flag_duplicate_subtrees ~forest articles; let () = let@ article = List.iter @~ articles in State.plant_resource ~forest T.(Article article) in + State.set_duplicate_diagnostics forest; (jobs, errors) let eval_only (uri : URI.t) (forest : State.t) = @@ -246,14 +285,19 @@ let eval_only (uri : URI.t) (forest : State.t) = | Loaded | Parsed | Evaluated -> assert false | Expanded -> begin let result, errors = Eval.eval_tree ~config ~uri t in - Diagnostics_store.set forest.diagnostics uri errors; + Diagnostics_store.replace forest.diagnostics uri Eval errors; match result with - | None -> (errors, []) + | None -> + State.set_owned_subtrees ~forest ~owner:uri []; + State.set_duplicate_diagnostics forest; + (errors, []) | Some {articles; jobs} -> + flag_duplicate_subtrees ~forest ~owner:uri articles; let () = let@ article = List.iter @~ articles in State.plant_resource ~forest (Article article) in + State.set_duplicate_diagnostics forest; (errors, jobs) end end diff --git a/lib/compiler/State.ml b/lib/compiler/State.ml index 74ba039..f899c41 100644 --- a/lib/compiler/State.ml +++ b/lib/compiler/State.ml @@ -19,6 +19,9 @@ type t = { dev: bool; config: Config.t; index: Tree.t URI.Tbl.t; + paths: string URI.Tbl.t; + mutable duplicate_uris: URI.Set.t; + subtree_owners: URI.t list URI.Tbl.t; last_good_articles: T.content T.article URI.Tbl.t; diagnostics: Diagnostics_store.t; mutable broken_links: URI.Set.t; @@ -43,6 +46,42 @@ let config forest = forest.config let env forest = forest.env let base forest = forest.config.url let iter_diagnostics f forest = Diagnostics_store.iter forest.diagnostics f + +let set_duplicate_diagnostics forest = + let origin_exists uri = + match URI.Tbl.find_opt forest.paths uri with + | Some path -> Result.is_ok (Eio_util.path_of_file ~env:forest.env path) + | None -> false + in + let live, gone = URI.Set.partition origin_exists forest.duplicate_uris in + forest.duplicate_uris <- live; + begin + let@ uri = URI.Set.iter @~ gone in + URI.Tbl.remove forest.paths uri; + Diagnostics_store.clear_phase forest.diagnostics uri Duplicate + end; + let@ uri = URI.Set.iter @~ live in + Diagnostics_store.set forest.diagnostics uri [Error.duplicate_tree ~uri] + +let set_owned_subtrees ~forest ~owner uris = + URI.Tbl.replace forest.subtree_owners owner (List.sort_uniq URI.compare uris) + +let owned_subtrees forest owner = + Option.value (URI.Tbl.find_opt forest.subtree_owners owner) ~default:[] + +let check_and_register_origin state uri origin = + match URI.Tbl.find_opt state.paths uri with + | Some existing when existing <> origin -> + state.duplicate_uris <- URI.Set.add uri state.duplicate_uris + | _ -> URI.Tbl.replace state.paths uri origin + +let register_path state uri tree = + match tree with + | Tree {phase = Loaded; document = Some doc; _} -> + check_and_register_origin state uri + (Lsp.Uri.to_path (Lsp.Text_document.documentUri doc)) + | _ -> () + let imports forest = forest.import_graph let make ~(env : Eio_unix.Stdenv.base) ~(config : Config.t) ~(dev : bool) @@ -59,6 +98,9 @@ let make ~(env : Eio_unix.Stdenv.base) ~(config : Config.t) ~(dev : bool) dev; config; index; + paths = URI.Tbl.create 1000; + duplicate_uris = URI.Set.empty; + subtree_owners = URI.Tbl.create 1000; last_good_articles = URI.Tbl.create 1000; diagnostics; import_graph; @@ -82,20 +124,12 @@ module Syntax = struct let ( .={} ) state uri = URI.Tbl.find_opt state.index uri let ( .={}<- ) state uri tree = + register_path state uri tree; let wrote = match state.={uri} with - | None -> - URI.Tbl.replace state.index uri tree; - true | Some existing when Tree.is_asset tree && Tree.is_asset existing -> false - | Some existing -> - if Tree.same_revision tree existing && Tree.equal_phases tree existing - then begin - Diagnostics_store.set state.diagnostics uri - [Error.duplicate_tree ~uri]; - URI.Tbl.replace state.index uri tree - end - else URI.Tbl.replace state.index uri tree; + | _ -> + URI.Tbl.replace state.index uri tree; true in if wrote then begin @@ -337,6 +371,14 @@ let plant_resource : let uri = URI.canonicalise uri in (* Seems dodgy if this isn't already canonical! *) Graphs.register_uri uri; + begin match resource with + | T.Article {frontmatter = {designated_parent = Some parent_uri; _}; _} -> + let root_uri = parent_file_uri ~forest parent_uri in + Option.iter + (check_and_register_origin forest uri) + (URI.Tbl.find_opt forest.paths root_uri) + | _ -> () + end; begin let@ host = Option.iter @~ URI.host uri in let base = URI.make ?scheme:(URI.scheme uri) ~host () in diff --git a/lib/core/Tree.ml b/lib/core/Tree.ml index a858de6..0034e20 100644 --- a/lib/core/Tree.ml +++ b/lib/core/Tree.ml @@ -138,16 +138,7 @@ let phase : type a. a tree -> phase = function | Parsed -> Phase Parsed | Expanded -> Phase Expanded | Evaluated -> Phase Evaluated - end - -let equal_phases = - fun (Tree {phase = phase1; _}) (Tree {phase = phase2; _}) -> - match (phase1, phase2) with - | Loaded, Loaded -> true - | Parsed, Parsed -> true - | Expanded, Expanded -> true - | Evaluated, Evaluated -> true - | _ -> false + end let document : t -> document option = function | Tree {document; _} as t -> document @@ -157,15 +148,6 @@ let of_doc : Lsp.Text_document.t -> t = let uri = Lsp.Text_document.documentUri document in Tree {phase = Loaded; tree = (); document = Some document} -let same_revision (t1 : t) (t2 : t) : bool = - match (document t1, document t2) with - | Some d1, Some d2 -> - Lsp.Uri.equal - (Lsp.Text_document.documentUri d1) - (Lsp.Text_document.documentUri d2) - && Lsp.Text_document.version d1 = Lsp.Text_document.version d2 - | _ -> compare (source t1) (source t2) = 0 - let syn = function {tree; _} -> tree.nodes let get_syn = function diff --git a/lib/language_server/Diagnostics.ml b/lib/language_server/Diagnostics.ml index 06d87b6..3b2be78 100644 --- a/lib/language_server/Diagnostics.ml +++ b/lib/language_server/Diagnostics.ml @@ -38,7 +38,6 @@ let render_diagnostics (forest : State.t) (uri : URI.t) : match State.get_article ~forest uri with | None -> [] | Some article -> - let pipeline_errors = Diagnostics_store.get forest.diagnostics uri in let errors = Link_checker.verify_links ~forest article in let own = ref [] in let foreign : (Lsp.Uri.t option * Error.t list ref) URI.Tbl.t = @@ -54,11 +53,12 @@ let render_diagnostics (forest : State.t) (uri : URI.t) : | None -> URI.Tbl.add foreign owner (lsp_uri_of_error error, ref [error]) end end; - Diagnostics_store.set forest.diagnostics uri - (pipeline_errors @ List.rev !own); + Diagnostics_store.replace forest.diagnostics uri Link_check + (List.rev !own); URI.Tbl.fold (fun owner (lsp_uri, errs) acc -> - Diagnostics_store.set forest.diagnostics owner (List.rev !errs); + Diagnostics_store.replace forest.diagnostics owner Link_check + (List.rev !errs); match lsp_uri with | Some lsp_uri -> (owner, lsp_uri) :: acc | None -> acc) @@ -98,13 +98,10 @@ let store_errors_by_owner (store : Diagnostics_store.t) ~default errors = | Some errs -> errs := error :: !errs | None -> URI.Tbl.add subtrees owner (ref [error]) end; - Diagnostics_store.set store default - (Diagnostics_store.get store default @ List.rev !own); + Diagnostics_store.replace store default Link_check (List.rev !own); URI.Tbl.fold (fun owner errs acc -> - (* A subtree won't produce compilation errors of its own, so we - overwrite the diagnostics.*) - Diagnostics_store.set store owner (List.rev !errs); + Diagnostics_store.replace store owner Link_check (List.rev !errs); owner :: acc) subtrees [] @@ -115,8 +112,13 @@ let compute (document : Lsp.Text_document.t) = let uri = URI.of_lsp_uri ~base:forest.config.url lsp_uri in let foreign_targets = render_diagnostics forest uri in let by_lsp_uri : (Lsp.Uri.t, Error.t list) Hashtbl.t = Hashtbl.create 4 in + let owned_diagnostics = + List.concat_map + (Diagnostics_store.get forest.diagnostics) + (State.owned_subtrees forest uri) + in Hashtbl.replace by_lsp_uri lsp_uri - (Diagnostics_store.get forest.diagnostics uri); + (Diagnostics_store.get forest.diagnostics uri @ owned_diagnostics); begin let@ owner, owner_lsp_uri = List.iter @~ foreign_targets in let existing = diff --git a/test/duplicate-tree-dispersed.t b/test/duplicate-tree-dispersed.t new file mode 100644 index 0000000..89cbd6a --- /dev/null +++ b/test/duplicate-tree-dispersed.t @@ -0,0 +1,12 @@ + $ forester init > /dev/null + + $ cat > trees/one.tree << EOF + > \subtree[dupaddr]{} + > EOF + + $ cat > trees/two.tree << EOF + > \subtree[parent]{\subtree[dupaddr]{}} + > EOF + + $ forester build 2>&1 | grep duplicate_tree + error[duplicate_tree]: duplicate tree https://www.my-great-forest.net/dupaddr/ diff --git a/test/duplicate-tree-distinct-paths.t b/test/duplicate-tree-distinct-paths.t new file mode 100644 index 0000000..2043dc2 --- /dev/null +++ b/test/duplicate-tree-distinct-paths.t @@ -0,0 +1,19 @@ + $ mkdir a b + $ cat > forest.toml << EOF + > [forest] + > trees = ["a", "b"] + > assets = [] + > home = "dup" + > url = "https://www.my-great-forest.net/" + > EOF + + $ cat > a/dup.tree << EOF + > \title{From a} + > EOF + + $ cat > b/dup.tree << EOF + > \title{From b} + > EOF + + $ forester build 2>&1 | grep duplicate_tree + error[duplicate_tree]: duplicate tree https://www.my-great-forest.net/dup/ diff --git a/test/duplicate-tree-same-file.t b/test/duplicate-tree-same-file.t new file mode 100644 index 0000000..7928379 --- /dev/null +++ b/test/duplicate-tree-same-file.t @@ -0,0 +1,9 @@ + $ forester init > /dev/null + + $ cat > trees/index.tree << EOF + > \subtree[dupaddr]{\title{First}} + > \subtree[dupaddr]{\title{Second}} + > EOF + + $ forester build 2>&1 | grep duplicate_tree + error[duplicate_tree]: duplicate tree https://www.my-great-forest.net/dupaddr/