Action looped recommandations
Completed
Hello !
I'm running ReSharper 2023.2.1
I have the following test code in C# :
[Fact]
public void ShouldNotAllowEmptyComputerName()
{
static void Action() => _ = new Folder(string.Empty, Path);
ArgumentNullException exception = Assert.Throws<ArgumentNullException>(Action);
exception.Message.Should().Be("Le nom de l'ordinateur est obligatoire. (Parameter 'computerName')");
}R# suggests to put the action at the end :
[Fact]
public void ShouldNotAllowEmptyComputerName()
{
ArgumentNullException exception = Assert.Throws<ArgumentNullException>(Action);
exception.Message.Should().Be("Le nom de l'ordinateur est obligatoire. (Parameter 'computerName')");
return;
static void Action() => _ = new Folder(string.Empty, Path);
}This creates a https://rules.sonarsource.com/csharp/RSPEC-3626/
Removing the return command :
[Fact]
public void ShouldNotAllowEmptyComputerName()
{
ArgumentNullException exception = Assert.Throws<ArgumentNullException>(Action);
exception.Message.Should().Be("Le nom de l'ordinateur est obligatoire. (Parameter 'computerName')");
static void Action() => _ = new Folder(string.Empty, Path);
}creates a R# suggestion : ‘Add explicit return’
On my original code, AI Assistant tells me : “This code is already written in a correct and efficient way. It follows the Arrange-Act-Assert pattern commonly used in unit testing.”
On the latest, it tells me : “This code is correct, but it might be more readable if the Action function is defined before it's used.”
While this is in no way blocking, should R# not suggest to move the code in the original place ?
Thank you !
Please sign in to leave a comment.
Hello, VTI
You can read more about this particular recommendation and its motivation in our blog post.
ReSharper/Rider recommends placing local functions after the 'return' statement to enhance code readability by separating executable code from local functions. It's often clear what local functions do from their names and arguments so you can understand the containing method without checking their actual implementations. This is akin to reading a method without first checking the implementation of every private method it might call.
While this argument may not seem compelling for a method that is only three lines long, it might be beneficial to maintain a consistent code style across your code base.
SonarSource inspection may not consider the readability improvements when the jump statement is located before a local function. Without an explicit return, developers have to scroll through local functions at the end of a method just to check that there's no executable code after them. ReSharper/Rider doesn't consider jump statements before local functions redundant, but third-part analyzers are out of our control.
Regarding the AI assistant recommendation - at this stage of AI development the assistant is in beta access and we do not consider its recommendations a rigid style guide that must be enforced.
You can follow its recommendations if you agree with them and think they are beneficial for code readability in your case.
However, AI suggestions might change depending on the underlying third-party model and the exact code you are using. E.g. in this particular case it might be tailored to the particular method you asked the AI assistant about. Since our current inspection set ensures that the code style is consistent across your projects, it relies on deterministic criteria instead, which might not match AI in every case.
You can also configure ReSharper/Rider inspections from the alt+enter menu if you prefer to ignore a particular recommendation. Code style is ultimately a matter of personal preference.
Hello Andrey !
Great answer ! Thank you very much and thanks for the blog link.
I fully agree with the rule and the how it helps with the overall readability.
I'll see to silence this particular sonar rule for now.