roslynator icon indicating copy to clipboard operation
roslynator copied to clipboard

Contribution proposal and discussion

Open AndreiShenets opened this issue 8 months ago • 4 comments

Hi,

I would like to contribute to the project and add a few things that I miss.

The first thing I would like to add a setting to RCS0053. The setting will force to put closing bracket on a new line. It might affect RCS0027 or requires a new formatter.

Examples:

diagnostic

void M(
    object x,
    object y, object z)
{
}

fix

void M(
    object x,
    object y, 
    object z
)
{
}

diagnostic

if (x &&
    y &&
    z)
{
}

fix

if (
    x
    && y
    && z
)
{
}

The second thing I would like to add is a diagnostic and fix to make code structurally honest. What structural honesty means described here

Examples:

diagnostic

Dictionary<string, NewItemType> itemDictionary = state.Where(item => item.Name != null)
    .Select(item => new NewItemType()
    {
        Name = item.Name
    })
    .ToDictionary(item => 
    {
        string key = item.Name;
        ...
        return key;
    },
    item => item,StringComparer.Ordinal);

fix

Dictionary<string, NewItemType> itemDictionary = 
    state
        .Where(item => item.Name != null)
        .Select(
            item => 
                new NewItemType()
                {
                    Name = item.Name
                }
        )
        .ToDictionary(
            item => 
            {
                string key = item.Name;
                ...
                return key;
            },
            item => item,
            StringComparer.Ordinal
        );

Please advise.

AndreiShenets avatar Mar 16 '25 12:03 AndreiShenets

Hi @josefpihrt,

I think you thought that I request the feature htat I described above. But I would like to implement it and therefore discuss it as the Contributor guide of the project requires.

Could you please guide me through the discussion process please? How are we going to do it? When do you think you have time for that? If you need time to think about it please let me know how long should I wait before next time pinging you. Without any information I don't know what I can expect.

Thanks a lot.

AndreiShenets avatar Mar 20 '25 07:03 AndreiShenets

Hey, sorry for the delay, I'm too busy. I'll try take a look during the weekend.

josefpihrt avatar Mar 20 '25 07:03 josefpihrt

Regarding the first proposal: I'd say that this should be a new analyzer, which may affect how code fix is applied for some analyzers like RCS0053 or RCS0027. Generally it's a good idea and you are more than welcome to create MR for it.

Regarding the second proposal: I think need to more precisely define what would the analyzer do and write it down before we should proceed. Also, I see some similarities with https://josefpihrt.github.io/docs/roslynator/analyzers/RCS0054.

josefpihrt avatar Mar 24 '25 20:03 josefpihrt

Thanks, I will start to prepare the new analyzer + changes for RCS0053 and RCS0027.

Regarding the other analyzer idea. Let me try to explain it with picture below. But first I would like to mention that I am thinking about having following modes for the analyzer Disabled | Tolerant | Strict. I will show results of formatting for each mode below.

If I do not miss something there are only two cases when it should be applied

  1. Lambdas / Delegates
  2. Object creation

Let's consider a simple case with lambdas

Initial not formatted code samples:

Image

Image

The problem with it is that head and body located in different places. To identify what is passed inside you need to look at too places and do a mental effort to match them and concatenate. Depending on names length it can be harder or easier but is always the mental effort. That's why I call it structurally dishonest: there are no clear language construction boundaries, you cannot select them with a rectangle without touching parent structures or "you cannot point to a structure with one finger"

The new analyzer should fix it. The result of its work in Tolerant mode

Image

Image

The result is structurally honest / consistent for the lambdas. The mode is Tolerant because formally there is one more structurally inconsistent element in the first case, still the readability is much better from my point of view.

Image

In Strict mode result will be following that is completely structurally correct.

Image

Image

Now let's consider a simple case for new objects

Initial not formatted code samples are:

Image

Image

The result of formatting in Tolerant mode is:

Image

Image

The results of formatting in `Strict mode is:

Image

Image

And Now let's consider the complex example from my initial message

Initial not formatted code

Image

Fixed code in both Tolerant and Strict modes because it is a complex construction

Image

I hope my explanations make the proposal clearer.

AndreiShenets avatar Mar 25 '25 08:03 AndreiShenets