Remove ! from Microsoft.Extensions.DependencyModel

[maxkoshevoi]

Description

This is a blanket issue for all ! operations that were added in #57445 (mainly in 57dee24)
They should be removed or reviewed.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @eerhardt
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

This is a blanket issue for all ! operations that were added in #57445 (mainly in 57dee24)
They should be removed or reviewed.

Author:	maxkoshevoi
Assignees:	-
Labels:	area-DependencyModel, untriaged
Milestone:	-

[AnudeepGunukula]

Hi @eerhardt @maxkoshevoi . i am interested in this project
i would like to work on this issue.

[eerhardt]

Sounds good. I assigned it to you.

[AnudeepGunukula]

Hi @eerhardt . i had made a pr that might solve this issue
Kindly review it and please let me know the changes if needed.
Thanks in advance

[filipjelic]

@maxkoshevoi @eerhardt If you all agree, I'd like to take over this issue?

[eerhardt]

Thanks, @filipjelic. I am assigning it to you.

[filipjelic]

@eerhardt Just a bit more information here...
Should the approach be to have a default value (empty string) if GetString returns null, or writing an additional TryGetString as part of Utf8JsonReader.TryGet.cs ?

[eerhardt]

As long as string.Empty and <null> don't need to be handled differently, that approach might work.

[filipjelic]

After reading the discussion I see the way you proposed (having ReadTarget accept nullable string) which is the most simplest.
Is that agreed way or should the <null> handled with an exception?

[eerhardt]

The heart of the problem (as far as I know) is that the DependencyModel code that reads a .deps.json file doesn't handle cases where a required strings are not provided in the file. It reads a string, stores it, and then later uses it as if it were not null. This can result in NullReferenceExceptions occurring.

We need to figure out a good approach to take when required strings are not in the file. I would guess the approach would be:

1. If a string is required, and not in the file, throw an exception stating "the required field 'XX' was not specified".
2. If a string is not required, ensure that it is typed as a string? everywhere.

[filipjelic]

After reading this resource
It seems the following objects are required:

Main .deps.json elements:

• runtimeTarget
• targets
• libraries

targets section (.deps.json)

• framework name (can contain also Runtime Identifier)
• libraries > target

libraries section (.deps.json)

• type

I'll re check my PR and update it accordingly.

[maxkoshevoi]

Should this be closed since #60842 is merged?

[eerhardt]

Yes! The PR wasn't linked to this issue as "fixes". Closing.