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

Collection expressions are suggested for types where it doesn't make sense, and their error messages show inconsistent types #73655

Open
cremor opened this issue May 23, 2024 · 6 comments

Comments

@cremor
Copy link

cremor commented May 23, 2024

Version Used:

  • VS 17.9.6 with SDK 8.0.204
  • VS 17.11 Preview 1 with SDK 8.0.300
  • VS 17.11 Preview 1 with SDK 9.0.100-preview.4.24267.66
  • sharplab.io "Default"

Steps to Reproduce:
See comments in the sample:

using System;
using System.Collections;
using System.Collections.Generic;

namespace ConsoleApp1;

internal class Program
{
    private readonly InlineValidator<DateTime> _validator;

    public Program()
    {
        // This shows "IDE0028 Collection initialization can be simplified", but only if the Add method below exists.
        // Should it really show this message? I think using a collection expression here is very weird.
        _validator = new InlineValidator<DateTime>();

        // Applying the code fix results in the following working, but weird, code.
        // This still compiles if the Add method below does not exist.
        _validator = [];

        // This calls the Add method below, but should it?
        // The type of the parameter is something completely different than the type of the IEnumerable element.
        // If the Add method below does not exist, then this shows error "CS1061 'InlineValidator<DateTime>' does not
        // contain a definition for 'Add' and no accessible extension method 'Add' accepting a first argument of type
        // 'InlineValidator<DateTime>' could be found".
        // This makes even less sense. Why does it try to find an Add method with parameter type
        // InlineValidator<DateTime>? Shouldn't it only be parameter type DateTime or IValidationRule (depending on
        // whether it should be the generic type of the class itself, or the IEnumerable type)?
        _validator = [null!];

        // This shows error "CS0029 Cannot implicitly convert type 'int' to 'ConsoleApp1.IValidationRule'.
        // This is different from the line above (when the Add method exists). There the compiler passed null as the
        // Add method parameter type (Func). But now it tries to convert it to the IEnumerable element type? Why?
        //_validator = [1];
    }
}

public class InlineValidator<T> : IEnumerable<IValidationRule>
{
    public void Add(Func<int, bool> someFunc)
    {
        someFunc(1);
    }

    public IEnumerator<IValidationRule> GetEnumerator()
    {
        throw new NotImplementedException();
    }
    IEnumerator IEnumerable.GetEnumerator()
    {
        throw new NotImplementedException();
    }
}

public interface IValidationRule;

Sample in SharpLab

My actual project uses https://github.com/FluentValidation/FluentValidation. The sample contains simplified versions of the FluentValidation classes.

Diagnostic Id:
Initially "IDE0028: Collection initialization can be simplified".
But the following error messages are shown if you modify the sample as explained in the comments:

  • "CS1061: 'InlineValidator<DateTime>' does not contain a definition for 'Add' and no accessible extension method 'Add' accepting a first argument of type 'InlineValidator<DateTime>' could be found"
  • "CS0029: Cannot implicitly convert type 'int' to 'ConsoleApp1.IValidationRule"

Expected Behavior:
I don't expect to see "IDE0028: Collection initialization can be simplified" for new InlineValidator<DateTime>(). I think this should only be shown if the parameter type of the Add method matches the collection type.
I expect consistant parameter/element types shown in the error messages if you modify the sample as explained in the comments.

Actual Behavior:
See comments in the sample code.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label May 23, 2024
Copy link

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.

@cremor cremor changed the title Collection expressions are suggested for types where it doesn't make sense Collection expressions are suggested for types where it doesn't make sense, and their error messages show inconsistent types May 23, 2024
@RikkiGibson
Copy link
Contributor

Thanks for filing this issue.

  • Could you please confirm whether the issue still occurs in the latest VS/dotnet SDK preview?
  • Could you please include the SharpLab code directly in the issue description in a ```cs block?

@cremor
Copy link
Author

cremor commented May 24, 2024

Yes, I can still reproduce this with the latest preview versions of VS and the .NET SDK. I've updated the issue description to include this information. And I've also directly included the sample code as requested.

@RikkiGibson
Copy link
Contributor

I'm not sure right now whether to classify as a compiler or IDE (suggest using collection-exprs) issue. I'll have to spend a little more time paging in what's going on to do so.

@CyrusNajmabadi
Copy link
Member

@RikkiGibson this is likely on hte IDE side. However, the core issue is that there are no APis the compiler exposes to know anything here. So we basically have had to reimplement all the logic aronud the rules in the language to know if somethign is a legal collectoin-expr type. that wasn't tenable originally, and it gets even harder as we add more complex and involved rules for collection types. As mentioned originally, the IDE needs apis to know if something is a legal collection expr type. Since that logic already exists in the compiler, we wouldn't have to duplicate it and we could defer directly to that to know if we can even bother offering collection exprs at all.

@cremor
Copy link
Author

cremor commented May 29, 2024

this is likely on hte IDE side

Aren't the error messages coming from the compiler?

@genlu genlu removed the untriaged Issues and PRs which have not yet been triaged by a lead label Jun 5, 2024
@genlu genlu added this to the 17.11 milestone Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants