bug: Add improved support for single arguments in get_list_args function

[TommyE123]

Fixes

Initial issue experienced in an Azure Devops pipeline.

When setting the CSHARP_DOTNET_FORMAT_ARGUMENTS variable to point to a solution file within a subfolder, the Dotnet_Format Linter encounters an error due to incorrect argument parsing. For example, setting:

CSHARP_DOTNET_FORMAT_ARGUMENTS: ".\SolutionFolder\SolutionFile.sln"

results in the following error: (notice the \ has gone) Unrecognized command or argument “.SolutionFolderSolutionFile.sln".

Similarly, using forward slashes: CSHARP_DOTNET_FORMAT_ARGUMENTS: "./SolutionFolder/SolutionFile.sln"

results in: Unrecognized command or argument "./SolutionFolder/SolutionFile.sln".

The problem appears to arise when the solution file is in a subfolder and the argument is passed through the shlex.split method, which misinterprets the slashes.

Proposed Changes

To address this, I've modified the get_list_args function to check if the input string contains a space character before splitting it. If the string doesn't contain a space, it is assumed to be a single argument and is returned as a list containing the original string.

This change should hopefully (assuming its not been already lost before or after this method call) fix the issue when using a single argument for CSHARP_DOTNET_FORMAT_ARGUMENTS that includes a path with subfolders. However, if multiple arguments are required, further adjustments may be needed.

This change ensures that single arguments with paths are preserved correctly and passed as intended.

Benefits:

• Correct Handling of Single Arguments: The change ensures that single arguments, especially paths, are not split incorrectly, which prevents errors when these arguments are passed to the linter.
• Backward Compatibility: The change retains the existing logic for handling lists, empty strings, and None values, ensuring that the function's behavior remains consistent for other input types.
• Simplicity: The additional check is straightforward and does not introduce unnecessary complexity. It simply ensures that single arguments are preserved as-is if they do not contain spaces.
• Improve support for single-word arguments Although it does not address cases with multiple arguments, it significantly improves the function's reliability in common scenarios where only one argument is used.

PR UPDATE

After extensive further testing, I've identified several issues with the dotnet_format linters. While these issues require more targeted fixes within the dotnet_format code, this proposed change won't directly resolve them. However, this change is necessary for ultimately addressing these issues.

The change involves fixing some cases where shlex.split is unnecessarily called, which doesn't handle certain scenarios well. I believe it's better to merge this change separately since it's generic and will still resolve some issues.

I've created 70 unit tests to ensure the method behaves consistently before and after the change. These tests have highlighted a few scenarios that now work correctly and others that still fail. I appreciate some might not currently be in use or required however they still now don’t need to call shlex.split if not required to!

In parallel, I am working on addressing the specific issues within the csharp and vb dotnet_format code itself to provide a comprehensive solution to vastly improve the speed, relative paths and other issues found.

Does this approach sound acceptable, or would you suggest any changes?

regards Tom

Testing

Tests added with existing code:

Sub-tests for [tests.test_megalinter.config_test.config_test.test_get_list_args]: Total=70 Passed=57 Failed=13

Tests failing with new code:

Sub-tests for [tests.test_megalinter.config_test.config_test.test_get_list_args]: Total=70 Passed=66 Failed=4

Tests left passing after commenting out broken tests with new code:

Sub-tests for [tests.test_megalinter.config_test.config_test.test_get_list_args]: Total=66 Passed=66 Failed=0

Readiness Checklist

Author/Contributor

• [x] Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)

Reviewing Maintainer

• [x] Label as either automation, bug, documentation, enhancement, infrastructure, or performance

[echoix]

Wasn't there a bug recently in the dotnet cli/tools with relative solution paths, something like this? I remember hitting it, but not with visual studio. Maybe when listing packages. I remember it trying to be solved, and announced by Microsoft somewhere, but can't exactly re find that info.

How does this fix help when there is a space in the path?

[TommyE123]

Hi @echoix,

The space is not really relevant. It simply means the shlex.split method doesn't need to be called if there isn't one {as that means theres only 1 argument), which I think is removing the \ from the path.

When I run dotnet format .\SolutionFolder locally from the repository root, it formats fine. Although, I could be mistaken, and the issue might be happening elsewhere in the megalinter code. However, I was thinking it would be better not to try and split a single argument when it's not required.

This fix would apply to arguments in general, not just the 'dotnet format' command.

I've tried a few workarounds like escaping the argument, but they often end up crashing the method (see attached).

Hope this make sense? Let me know if you need any clarification or have additional thoughts.

Traceback Errors.txt

[nvuillam]

The best with arguments of type "list" is to use the YML list way

For example, in .mega-linter.yml file:

CSHARP_DOTNET_FORMAT_ARGUMENTS: 
  - ".\SolutionFolder\SolutionFile.sln"

The split is a not pretty workaround, but i'm not sure what would happen with your update if we had something like :

CSHARP_DOTNET_FORMAT_ARGUMENTS: 
  - "unique element with spaces"

[TommyE123]

Hi @echoix, I've updated the PR comments and believe this to now be ready to merge. Any questions or further tests you would like added please let me know.

Tom

[echoix]

With all that proof, before and after, all these test cases, it would be hard to ask for more. Clearly the state is better with than without.

Thanks again for another amazing work!

[nvuillam]

Impressive list of test cases indeed :)