[Style] Target-typed new in the codebase

[Youssef1313]

We have excessive use of target-typed new, which (IMO) makes the code much harder to read in many cases where the type isn't apparent.

I would consider switching the code style to even disallow its usage completely. It's a personal opinion though and there is no right/wrong answer here.

[nohwnd]

Sometimes I find it hard, sometimes I don't. thinking about it, it is maybe harder than it was with var before.

Additionally I would really like to see the use var when type is obvious relaxed. Because almost everywhere else tells me to not use var.

[Youssef1313]

Sometimes I find it hard, sometimes I don't. thinking about it, it is maybe harder than it was with var before.

It's not only about var, it's also about patterns like the following:

testfx/src/Platform/Microsoft.Testing.Extensions.VSTestBridge/ObjectModel/ObjectModelConverters.cs

Line 74 in aaa16e9

testNode.Properties.Add(new TestFileLocationProperty(testCase.CodeFilePath, new(new(testCase.LineNumber, -1), new(testCase.LineNumber, -1))));

which IMO should be really avoided.

[nohwnd]

I see. There I can still kind of guess what it is doing, but looking at the code without ide support or trying to quickly understand is definitely more difficult than if the whole type name was spelled out.

[SimonCropp]

Very much IMHO...
i still usually prefer target typed new. yes in some cases it is less readable. but i think there are better ways to make things clearer.

eg in the above code example i would write

var position = new LinePosition(testCase.LineNumber, -1);
testNode.Properties.Add(
    new TestFileLocationProperty(testCase.CodeFilePath, lineSpan: new(position, position)));

note it also removes a duplicate LinePosition instance

[nohwnd]

Only mentioned copilot to implement the suggestion provided by Simon, and nothing else. But it did not listen to me apparently.

[SimonCropp]

@nohwnd wouldnt it be better to have an overload of TestFileLocationProperty that accepts a single position?

[nohwnd]

Looks fine to me as is, better to keep the public api smaller rather than specialize it for that case. We are also not getting the column, and get a magic -1 number instead, even thought the item is probably not placed on unknown column, and does not take 0 characters.