From 68545369f007f467d07f2b56817db21aedb3def3 Mon Sep 17 00:00:00 2001 From: x1340 Date: Thu, 23 Oct 2025 01:19:51 +0300 Subject: [PATCH] feat: add diagnostic for non-markdown files and directories --- Marksman/Diag.fs | 95 +++++++++++++++++++++++++++++++--- Marksman/Misc.fs | 3 +- Tests/DiagTest.fs | 127 +++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 216 insertions(+), 9 deletions(-) diff --git a/Marksman/Diag.fs b/Marksman/Diag.fs index 3b17c424..3d6fb2b8 100644 --- a/Marksman/Diag.fs +++ b/Marksman/Diag.fs @@ -1,12 +1,16 @@ module Marksman.Diag +open System +open System.IO open Ionide.LanguageServerProtocol.Types open Marksman.Misc open Marksman.Names +open Marksman.Paths open Marksman.Doc open Marksman.Folder open Marksman.Workspace +open Marksman.Syms module Lsp = Ionide.LanguageServerProtocol.Types @@ -47,6 +51,58 @@ let checkNonBreakingWhitespace (doc: Doc) = [ NonBreakableWhitespace(whitespaceRange) ]) +// Check if a link target exists on file system (for non-markdown files/directories) +let tryResolveNonMarkdownPath (folder: Folder) (doc: Doc) (linkPath: string) : bool = + try + let decodedPath = linkPath.UrlDecode() + + // Handle file:// URIs + let pathStr = + if decodedPath.StartsWith("file://", StringComparison.OrdinalIgnoreCase) then + Uri(decodedPath).LocalPath + else + decodedPath + + // Resolve path + let absPath = + if pathStr.StartsWith('/') || pathStr.StartsWith('\\') then + // Check if it's already an absolute system path + if Path.IsPathRooted(pathStr) then + // Use as is + pathStr + else + // Combine with current file path. + let currentPath = (Folder.id folder).data |> RootPath.toSystem + Path.Combine(currentPath, pathStr.TrimStart('/', '\\')) + else + // Relative from document directory + let docDir = Doc.path doc |> AbsPath.toSystem |> Path.GetDirectoryName + Path.Combine(docDir, pathStr) + + let normalizedPath = Path.GetFullPath(absPath) + + // Check existence + File.Exists(normalizedPath) || Directory.Exists(normalizedPath) + with + | _ -> false + +// Check if a path is an external URL (consider file:// as internal URL) +let isExternalUrl (path: string) : bool = + Uri.IsWellFormedUriString(path, UriKind.Absolute) && + not (path.StartsWith("file://", StringComparison.OrdinalIgnoreCase)) + +// Check a non-markdown target and return diagnostic if it doesn't exist +let checkNonMarkdownTarget (folder: Folder) (doc: Doc) (linkEl: Element) (targetPath: string) (refOpt: option) : list = + if tryResolveNonMarkdownPath folder doc targetPath then + [] // Exists, no diagnostic + else + // Missing - create diagnostic + let refToUse = + match refOpt with + | Some r -> r + | None -> Ref.CrossRef(CrossRef.CrossDoc targetPath) + [ BrokenLink(linkEl, refToUse) ] + let checkLink (folder: Folder) (doc: Doc) (linkEl: Element) : seq = let exts = Folder.configuredMarkdownExts folder @@ -56,7 +112,21 @@ let checkLink (folder: Folder) (doc: Doc) (linkEl: Element) : seq = |> Option.bind Syms.Sym.asRef match ref with - | None -> [] + | None -> + match linkEl with + | ML { data = MdLink.IL(_, url, _) } -> + match url with + | Some { data = url } -> + let decodedUrl = UrlEncoded.decode url + if isExternalUrl decodedUrl then + [] // External URL - no diagnostic + else if Misc.isPotentiallyInternalRef exts decodedUrl then + [] // Markdown file - already handled by symbol system + else + // Non-markdown file/directory - check existence + checkNonMarkdownTarget folder doc linkEl decodedUrl None + | None -> [] + | _ -> [] | Some ref -> let refs = Dest.tryResolveElement folder doc linkEl |> Array.ofSeq @@ -72,13 +142,24 @@ let checkLink (folder: Folder) (doc: Doc) (linkEl: Element) : seq = | ML { data = MdLink.IL(_, url, _) } -> match url with | Some { data = url } -> - // Inline links to docs that don't look like a markdown file should not - // produce diagnostics - if Misc.isMarkdownFile exts (UrlEncoded.decode url) then - [ BrokenLink(linkEl, ref) ] + let decodedUrl = UrlEncoded.decode url + if Misc.isMarkdownFile exts decodedUrl then + [ BrokenLink(linkEl, ref) ] // markdown + else + checkNonMarkdownTarget folder doc linkEl decodedUrl (Some ref) + | _ -> [ BrokenLink(linkEl, ref) ] // non-markdown + | WL { data = wl } -> + match wl.doc with + | Some docNode -> + let docPath = WikiEncoded.decode docNode.data + if isExternalUrl docPath then + [] // External URL - no diagnostic + else if Misc.isMarkdownFile exts docPath then + [ BrokenLink(linkEl, ref) ] // markdown - diagnostic else - [] - | _ -> [ BrokenLink(linkEl, ref) ] + // Non-markdown file/directory - check existence + checkNonMarkdownTarget folder doc linkEl docPath (Some ref) + | None -> [ BrokenLink(linkEl, ref) ] | _ -> [ BrokenLink(linkEl, ref) ] else [ AmbiguousLink(linkEl, ref, refs) ] diff --git a/Marksman/Misc.fs b/Marksman/Misc.fs index 19ad7cf4..3ed8c6cb 100644 --- a/Marksman/Misc.fs +++ b/Marksman/Misc.fs @@ -169,7 +169,8 @@ let isPotentiallyMarkdownFile (configuredExts: seq) (path: string) : boo let isPotentiallyInternalRef (configuredExts: seq) (name: string) : bool = if Uri.IsWellFormedUriString(name, UriKind.Absolute) then - false + // Only file:// scheme is considered internal for absolute URIs + name.StartsWith("file://", StringComparison.OrdinalIgnoreCase) else isPotentiallyMarkdownFile configuredExts name diff --git a/Tests/DiagTest.fs b/Tests/DiagTest.fs index 80cc1491..dd8ded1b 100644 --- a/Tests/DiagTest.fs +++ b/Tests/DiagTest.fs @@ -9,6 +9,8 @@ open Marksman.Names open Marksman.Paths open Marksman.Doc open Marksman.Folder +open Marksman.Config +open Marksman.Text let entryToHuman (entry: Entry) = let lsp = diagToLsp entry @@ -73,7 +75,6 @@ let noDiagOnNonMarkdownFiles () = "## H2" "[](bad.md)" "[](another%20bad.md)" - "[](good/folder)" |] ) @@ -118,3 +119,127 @@ let noCrossFileDiagOnSingleFileFolders () = ], diag ) + +[] +let issue_429 () = + let testDir = "/tmp/marksman-test" + let testFile = "test.md" + + // Create test directory and files + System.IO.Directory.CreateDirectory(testDir) |> ignore + System.IO.Directory.CreateDirectory(System.IO.Path.Combine(testDir, "subfolder")) |> ignore + + // Create test files that should exist + System.IO.File.WriteAllText(System.IO.Path.Combine(testDir, "test.pdf"), "dummy pdf content") + System.IO.File.WriteAllText(System.IO.Path.Combine(testDir, "image.png"), "dummy image content") + System.IO.File.WriteAllText(System.IO.Path.Combine(testDir, "subfolder", "file.txt"), "dummy text content") + + // Create markdown document with links + let absTestDir = System.IO.Path.GetFullPath(testDir) + let content = $"""# Test Document + +## Links to existing files (relative paths) +- [Link to PDF](test.pdf) +- [Link to image](image.png) +- [Link to subfolder file](subfolder/file.txt) +- [[test.pdf]] +- [[image.png]] +- [[subfolder/file.txt]] + +## Links to existing files (absolute paths) +- [Absolute PDF]({absTestDir}/test.pdf) +- [Absolute image]({absTestDir}/image.png) +- [[{absTestDir}/test.pdf]] +- [[{absTestDir}/image.png]] +- [[{absTestDir}/subfolder/file.txt]] + +## Links to missing files (relative paths) +- [Missing PDF](missing.pdf) +- [Missing image](missing.png) +- [[missing.pdf]] +- [[missing-file]] + +## Links to missing files (absolute paths) +- [Absolute missing]({absTestDir}/missing.pdf) +- [[{absTestDir}/missing.pdf]] +- [[{absTestDir}/missing-file]] + +## External links +- [External](https://example.com) +- [External with file](https://example.com/file.pdf) +- [HTTP](http://example.com/file.pdf) +- [HTTPS](https://example.com/doc.pdf) +- [FTP](ftp://example.com/file.txt) +- [Mailto](mailto:test@example.com) +- [Data URI](data:text/plain;base64,SGVsbG8=) +- [[https://example.com/file.pdf]] +- [[http://example.com]] +- [[ftp://ftp.example.com/file.zip]] + +## file:// URIs (internal - should be checked) +- [File URI existing](file://{absTestDir}/test.pdf) +- [File URI missing](file://{absTestDir}/nonexistent.pdf) +""" + + let testPath = System.IO.Path.Combine(testDir, testFile) + System.IO.File.WriteAllText(testPath, content) + + // Create files with absolute paths + let absTestPath = System.IO.Path.GetFullPath(testPath) + let rootUri = $"file://{absTestDir}" + let folderId = UriWith.mkRoot rootUri + let docUri = $"file://{absTestPath}" + let docId = DocId(UriWith.mkRooted folderId (LocalPath.ofUri docUri)) + + let text = Text.mkText content + let doc = Doc.mk (ParserSettings.OfConfig(Config.Default)) docId None text + + let folder = Folder.multiFile "test-folder" folderId [doc] None + let diag = checkFolder folder |> diagToHuman + + // No diagnostic for external URLs (http, https, ftp, mailto, data) + let externalUrlErrors = diag |> List.filter (fun (_, msg) -> + msg.Contains("example.com") || + msg.Contains("test@example.com") || + msg.Contains("base64")) + + let externalErrorMsg = + if externalUrlErrors.IsEmpty then + "" + else + let errors = externalUrlErrors |> List.map (fun (f, m) -> $"{f}: {m}") |> String.concat "\n " + $"Should not have diagnostics for external URLs, but found:\n {errors}" + + Assert.True(externalUrlErrors.IsEmpty, externalErrorMsg) + + // Diagnostics for missing files (including file:// URI to missing file) + let missingOnly = diag |> List.filter (fun (_, msg) -> + msg.Contains("missing.pdf") || + msg.Contains("missing.png") || + msg.Contains("missing-file") || + msg.Contains("nonexistent.pdf")) + + // Should have diagnostics for missing files + Assert.NotEmpty(missingOnly) + + // Should NOT have diagnostics for existing files (test.pdf, image.png, subfolder/file.txt) + let existingFileErrors = diag |> List.filter (fun (_, msg) -> + (msg.Contains("test.pdf") || + msg.Contains("image.png") || + msg.Contains("subfolder/file.txt")) && + not (msg.Contains("missing"))) + + let errorMsg = + if existingFileErrors.IsEmpty then + "Should not have diagnostics for existing files" + else + let errors = existingFileErrors |> List.map (fun (f, m) -> $"{f}: {m}") |> String.concat "\n " + $"Should not have diagnostics for existing files, but found:\n {errors}" + + Assert.True(existingFileErrors.IsEmpty, errorMsg) + + // Clean up test directory + try + System.IO.Directory.Delete(testDir, true) + with + | _ -> () // Ignore cleanup errors