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

NullabilityInfoContext does not properly respect CodeAnalysis attribute on indexer property #63849

Closed
madelson opened this issue Jan 16, 2022 · 3 comments · Fixed by #64143
Closed

Comments

@madelson
Copy link
Contributor

Description

In most cases code analysis attributes like AllowNull are respected, but this is not true for indexer properties.

Reproduction Steps

public void Main()
{
        var context = new NullabilityInfoContext();
        var indexerInfo = context.Create(typeof(HasIndexer).GetProperty("Item")!);
       Console.WriteLine(indexerInfo.WriteState); // NotNull (should be Nullable)
}

    public class HasIndexer
    {
        [AllowNull] public string this[string? s]
        {
            get => default!;
            set { }
        }
    }

Expected behavior

The write state should be nullable because the following compiles without warnings:

new HasIndexer()["a"] = null;

Actual behavior

Write state is not null as if the attribute were not present.

Regression?

No response

Known Workarounds

No response

Configuration

VS 17.0.2, .NET 6, Windows 10 x64.

I don't have any reason to believe this is configuration specific.

Other information

CC @Shane32

@ghost
Copy link

ghost commented Jan 16, 2022

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

In most cases code analysis attributes like AllowNull are respected, but this is not true for indexer properties.

Reproduction Steps

public void Main()
{
        var context = new NullabilityInfoContext();
        var indexerInfo = context.Create(typeof(HasIndexer).GetProperty("Item")!);
       Console.WriteLine(indexerInfo.WriteState); // NotNull (should be Nullable)
}

    public class HasIndexer
    {
        [AllowNull] public string this[string? s]
        {
            get => default!;
            set { }
        }
    }

Expected behavior

The write state should be nullable because the following compiles without warnings:

new HasIndexer()["a"] = null;

Actual behavior

Write state is not null as if the attribute were not present.

Regression?

No response

Known Workarounds

No response

Configuration

VS 17.0.2, .NET 6, Windows 10 x64.

I don't have any reason to believe this is configuration specific.

Other information

CC @Shane32

Author: madelson
Assignees: -
Labels:

area-System.Reflection

Milestone: -

@danmoseley danmoseley added the untriaged New issue has not been triaged by the area owner label Jan 17, 2022
@danmoseley
Copy link
Member

Thanks for opening these nullable annotation bugs @madelson . Are you interested in offering a PR for any of them?

@madelson
Copy link
Contributor Author

@danmoseley yes I'm working on fixes for some of them with the idea of sending a PR.

@buyaa-n buyaa-n removed the untriaged New issue has not been triaged by the area owner label Jan 19, 2022
@buyaa-n buyaa-n added this to the 7.0.0 milestone Jan 19, 2022
madelson added a commit to madelson/runtime that referenced this issue Jan 22, 2022
In several cases, NullabilityInfoContext would return values which were
out of sync with the C# compiler's interpretation of the nullability
metadata. This fixes the following cases:
* The `NullablePublicOnly` attribute was not considered when analyzing
private constructor parameters.
* CodeAnalysis attributes on indexer properties were not recognized.
* CodeAnalysis attributes were not recognized in a `#nullable disabled`
context.
* Private value-typed members lacking nullable annotations were not
properly marked as nullable/notnull.
* Mixing CodeAnalysis attributes with opposite meanings (e. g.
`AllowNull` and `DisallowNull` produced the wrong result.
* Analysis of members inherited from generic base types did not
incorporate the nullable metadata associated with the inheritance.

Fix dotnet#63555
Fix dotnet#63846
Fix dotnet#63847
Fix dotnet#63848
Fix dotnet#63849
Fix dotnet#63853
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 22, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 26, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants