dotnet / docfx

Static site generator for .NET API documentation.
https://dotnet.github.io/docfx/
MIT License
4.06k stars 863 forks source link

Option to treat 'Invalid file link' as error instead of warning #7433

Open melanchall opened 3 years ago

melanchall commented 3 years ago

Operating System: Windows

DocFX Version Used: 2.58

Sometimes I forget to precede a link to an API entity with xref: which leads to warnings like these:

[21-06-19 09:42:16.151]Warning:[BuildCommand.BuildCore.Build Document.LinkPhaseHandlerWithIncremental.ConceptualDocumentProcessor.Save](articles/playback/Data-tracking.md#L100)Invalid file link:(~/articles/playback/Melanchall.DryWetMidi.Devices.Playback.TrackControlValue).
[21-06-19 09:42:16.151]Warning:[BuildCommand.BuildCore.Build Document.LinkPhaseHandlerWithIncremental.ConceptualDocumentProcessor.Save](articles/playback/Custom-playback.md#L16)Invalid file link:(~/articles/playback/Melanchall.DryWetMidi.Devices.Playback.OutputDevice).

And the message from docfx says all is fine (almost):

Build succeeded with warning.

I build my docs within CI, so I want a build failed when Invalid file link error occured, because from my point of view it's absolutely an error, not a just warning. I must fix it before publishing the docs.

So my question: is it possible to tell docfx to treat such situations as errors and thus fail the build (I suppose docfx should return non-zero exit code)?

Thanks, Max

jamiehankins commented 3 years ago

Do you build your docs from the command line or use the NuGet package for DocFX?

If you use the command-line, just add --warningsAsErrors.

If you use the NuGet package, here's a skelleton csproj file you can use as a template. I keep my DocFX stuff in a directory called docs. The docfx.json file is directly beside my csproj file. Note the last section, "DocFX settings".

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup>
    <ProjectReference Include="..\SupportLibrary\SupportLibrary.csproj" />
    <PackageReference Include="docfx.console" Version="2.58.0">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>

  <!-- Clean up DocFX output -->
  <Target Name="CleanDocFX" AfterTargets="DocClean" Condition="$(Configuration.Contains('All'))">
    <Message Text="Cleaning up DocFX generated documentation." Importance="high" />
    <RemoveDir Directories="$(ProjectDir)docs\_site\" />
    <ItemGroup Label="DocFX Artifacts to Clean">
      <FilesToDelete Include="$(ProjectDir)docs\api\*.yml" />
      <FilesToDelete Include="$(ProjectDir)docs\api\.manifest" />
      <FilesToDelete Include="$(ProjectDir)docs\log.txt" />
    </ItemGroup>
    <Delete Files="@(FilesToDelete)" />
  </Target>

  <PropertyGroup Label="DocFX settings">
    <LogFile>docs\log.txt</LogFile>
    <LogLevel>Warning</LogLevel>
    <DocParameters>--warningsAsErrors</DocParameters>
  </PropertyGroup>

</Project>
melanchall commented 3 years ago

@jamiehankins I use DocFX from the command line. Thanks for the --warningsAsErrors option. Unfortunately I can't find it on the official docs on DocFX: https://dotnet.github.io/docfx/tutorial/docfx.exe_user_manual.html. Maybe it would be good to add information about the option?

From what I've found on the web (here and here) the option should be used with true value:

--warningsAsErrors true

So it's not just a flag, right?

jamiehankins commented 3 years ago

Hi Maxim,

I'm not actually associated with the DocFX project. I'm just a guy that uses it like you do. I've fixed a couple of bugs and made pull requests, one of which was accepted and one of which is pending.

I would suggest editing the user manual and creating a pull request. The warningsAsErrors option was added in 2018 here: https://github.com/dotnet/docfx/pull/3230.

Thanks, Jamie

zdrawku commented 3 years ago

Is there a way to set the warningsAsErrors as property in the docfx.json file?

zdrawku commented 3 years ago

As a temporary solution I've managed incorporate the option to our build definition. We are using a custom approach that at the end spawns the actual command.

Handling on exit event do the trick of identifying the specific error code that related to Internal links/Hyperlink warningsAsErrors and we were able to return our own error message:

image

Core reference:

return spawn("docfx", ["build", `--warningsAsErrors`, `${path.normalize(getPath(docfxJsonPath))}`], { stdio: 'inherit' }).on('exit', (err) => { ...  } });

Equivalent to:

docfx build --warningsAsErrors D:/Dev/igniteui-docfx/en/docfx.json
DkSkydancer commented 1 year ago

It occurs to me, that the msbuild target needs to Check for exit code to fail the build when task calling docfx --warningsAsErros true exists with a non-zero exist code. but it doesn't today: https://github.com/dotnet/docfx/blob/d1116472194ad413b2d8365ccc122eef752e1a4b/src/nuspec/docfx.console/build/docfx.console.targets#L49-L55

KalleOlaviNiemitalo commented 1 year ago

The Exec task of MSBuild already checks the exit code, unless the target sets IgnoreExitCode="true".

https://github.com/dotnet/msbuild/blob/971bf70db73ebd5d5e1dde5eb27b8265be9f3169/src/Utilities/ToolTask.cs#L1527-L1529 https://github.com/dotnet/msbuild/blob/971bf70db73ebd5d5e1dde5eb27b8265be9f3169/src/Tasks/Exec.cs#L330-L353

filzrev commented 6 months ago

Latest version of docfx supports rules feature. https://dotnet.github.io/docfx/reference/docfx-json-reference.html?rules

So it can set custom rules for specific warning code. For original issue case. It can change warning to error with following settings.

"rules": {
    "InvalidFileLink": "error"
  }