Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partial attempt at adding nullable reference annotations (#4007) #6461

wants to merge 1 commit into
base: dev
Choose a base branch


Copy link

@kevin-montrose kevin-montrose commented Aug 27, 2020

I'm using docfx in one my libraries, and finally got around to digging into why nullable reference annotations aren't included in the generated YAML/HTML. I see there's an old-ish issue (#4007) that mentions this as well.

This is a draft because, well, it's not finished. For the life of me I cannot figure out why what's ending up in the YAML isn't ended up in the generated HTML (using the default template). I would like to finish this work and submit a proper PR, but after a couple days of false starts I'm hopeful someone more familiar with the code can tell me what I'm missing.

Where things are, concretely.

I currently have one test case, and it's generating "correct enough for now" YAML - specifically in MetadataItem.Syntax.Content. I suspect there are are still some real issues, and there are definitely still some TODOs - but things are far enough along for me to try testing on my library.

However, when I do test against my library I get the new YAML but not the HTML I'm expecting.


For a declaration like so (source):

public static IAsyncEnumerable<TRow> EnumerateAsync<TRow>(
            TextReader reader,
            Options? options = null,
            object? context = null,
            CancellationToken cancel = default

I'll get a chunk of YAML including (full chunk in this gist):

    content: public static IAsyncEnumerable<TRow> EnumerateAsync<TRow>(TextReader reader, Options? options = null, object? context = null, CancellationToken cancel = null)

But the HTML is

  <h5 class="decalaration">Declaration</h5>
  <div class="codewrapper">
    <pre><code class="lang-csharp hljs">public static IAsyncEnumerable&lt;TRow&gt; EnumerateAsync&lt;TRow&gt;(TextReader reader, Options options = null, object context = null, CancellationToken cancel = null)</code></pre>

Which looks like so:

I'm building docfx with the following command:
.\build.ps1 -skipTests -configuration Debug

And pulling out what ends up in \bin\Debug\. I did see there's a target directory that gets built too, but trying its contents has made no difference either.

Cesil's docfx.json is in its repo, but I'll reproduce it here.

  "metadata": [
      "src": [
          "files": [
      "dest": "api",
      "disableGitFeatures": false,
      "disableDefaultFilter": false
  "build": {
    "globalMetadata": {
        "_enableSearch": false,
        "_disableToc": true
    "content": [
        "files": [
    "resource": [
        "files": [
    "overwrite": [
        "files": [
        "exclude": [
    "dest": "docs",
    "globalMetadataFiles": [],
    "fileMetadataFiles": [],
    "template": [
    "postProcessors": [],
    "markdownEngineName": "markdig",
    "noLangKeyword": false,
    "keepFileLink": false,
    "cleanupCacheHistory": false,
    "disableGitFeatures": false

Poking around and liberally dropping breakpoints, best I can tell the syntax bits from the YAML are being loaded up correctly. Something is obviously off... somewhere - my gut tells me I've either messed up a build step, or am missing somewhere where syntax is transformed in the default templates (which would imply I need to either change a template, or update additional fields in the YAML).

Any guidance on what I'm missing?

Microsoft Reviewers: Open in CodeFlow

this approach seems fine, if a little awkward, because null annotations can double on nullable _value_ types
@superyyrrzz superyyrrzz added the v2 label Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet

Successfully merging this pull request may close these issues.

None yet

2 participants