Facets nullable (#311 & #307)

[nikcio]

This adds the nullable feature to Examine and the Facets feature. This means that this PR is a super set of #311 and should be merged after the facets feature. This is PR also includes the changes in #307 which adds multitargeting which is necessary to support nullable.

To see the changes in this PR before merging #311 and #307 see this commit

What changed

• Added <Nullable>enable</Nullable> to add the nullable feature
• Added <LangVersion>9</LangVersion> to support some of the more useful nullable features
• Disabled nullable warnings in netstandard2.0 - This version doesn't support nullable that well and it's way easier to omit the warnings. The nullable features that netstandard2.0 have will also be included without fixing the warnings and these features will also be correct when fixing similar places with the warnings from netstandard2.1 and net6.0

<!-- Disable the nullable warnings when compiling for .NET Standard 2.0 -->
  <PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'">
    <NoWarn>$(NoWarn);nullable</NoWarn>
  </PropertyGroup>

• Added so many question marks

Approch

This PR does it's best to add the nullable feature without changing any logic, therefore, if a property was able to be null it's been marked that way.

TODOs

Some places in the code can't comply with the nullable feature without changing the code behaviour (I would like help deciding what to do with these. It doesn't have to be part of this PR):

Note: The tasks can be found with an attached // TODO: marker in the PR code

• src/Examine.Core/OrderedDictionary.cs - TVal IDictionary<TKey, TVal>.this[TKey key] It's common for this to throw if the key is not found but this returns null instead.
• src/Examine.Lucene/Providers/LuceneIndex.cs - public TrackingIndexWriter IndexWriter Here it's possiable for _writer to be null. Maybe we should throw if this happens?
• src/Examine.Lucene/Search/LuceneSearchQueryBase.cs - protected virtual Query GetFieldInternalQuery(string fieldName, IExamineValue fieldValue, bool useQueryParser) Here a null check in one of the cases tells the compiler that this value can be null at some point. If this is the case then the methods relevant for setting this value should be marks as possiable to return null.

[nikcio]

@Shazwazza This PR has also been updated 😄

[Shazwazza]

Awesome :) Have left some comments.

[nikcio]

@Shazwazza Super I've updated the PR we only need 1 TODO then. What do we do with : src/Examine.Lucene/Providers/LuceneIndex.cs - public TrackingIndexWriter IndexWriter Here it's possiable for _writer to be null. Maybe we should throw if this happens?

[nikcio]

Hey @Shazwazza can I do anything to move this PR forward? 😁

[Shazwazza]

Just the nudge is good :) Sorry for the wait. I'm not sure why PRs aren't doing normal builds and reporting here, I guess something else that needs fixing.

I'll get this merged and we can iterate from there.

[nikcio]

@Shazwazza It might be the branch filter for the pull request trigger in the build.yml workflow that keeps the build from running. If you look in the action history there isn't any action runs from a PR

[Shazwazza]

@nikcio maybe this syntax isn't valid?

Examine/.github/workflows/build.yml

Line 12 in bbc436a

pull_request:

[nikcio]

@Shazwazza Yeah thats my thought. And it you want to build on all prs you can just delete the branch: '*' bit. But are your build action also publishing to nuget? Thats what I can see you might want a branch filter on that part

[Shazwazza]

Right. I published to GH packages but I'm going to remove that since nobody uses that.

[Shazwazza]

I've updated the logic in master branch for both build and docs builds. We'll see if that works on next PR.