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

Add Roslynator linter #3155

Merged
merged 8 commits into from
Nov 29, 2023
Merged

Add Roslynator linter #3155

merged 8 commits into from
Nov 29, 2023

Conversation

bdovaz
Copy link
Collaborator

@bdovaz bdovaz commented Nov 23, 2023

Fixes #2269

Proposed Changes

Dotnet restore command:

In the end I put it in the "process_linter" method because it is the only one I see that I could do it in.

Related to tests:

I had to add 2 specific variables to the descriptors for the format_fix test.

  • test_format_fix_file_extensions: specific extensions to this test
  • test_format_fix_regex_exclude: exclusions specific to this test.

You may ask, why?

Well, because this linter takes as input ".csproj" files but when doing format/fix it actually modifies the ".cs" files which are the scripts contained in the project (.csproj).

The value that I have put to each variable is the following:

  • test_format_fix_file_extensions: .cs -> so that the diff that does the format_fix test only takes into account the files it actually alters.
  • test_format_fix_regex_exclude: /bin/|/obj/ -> as it has to run a dotnet restore previously, it generates the folders /bin, /obj and these folders do not have to be taken into account.

Besides in this test I have simplified the file filtering logic and I have used the "core" utils.filter_files method to avoid duplicated and error prone logic

Copy link
Contributor

github-actions bot commented Nov 23, 2023

🦙 MegaLinter status: ❌ ERROR

Descriptor Linter Files Fixed Errors Elapsed time
✅ BASH bash-exec 5 0 0.05s
✅ BASH shellcheck 5 0 0.15s
✅ BASH shfmt 5 0 0 0.48s
✅ COPYPASTE jscpd yes no 4.14s
✅ DOCKERFILE hadolint 127 0 15.59s
✅ JSON eslint-plugin-jsonc 23 0 0 3.86s
✅ JSON jsonlint 21 0 0.25s
✅ JSON v8r 23 0 18.71s
✅ MAKEFILE checkmake 1 0 0.01s
⚠️ MARKDOWN markdownlint 259 0 11 7.43s
✅ MARKDOWN markdown-link-check 259 0 8.28s
✅ MARKDOWN markdown-table-formatter 259 0 0 51.77s
✅ OPENAPI spectral 1 0 1.65s
⚠️ PYTHON bandit 206 61 4.23s
✅ PYTHON black 206 0 0 4.5s
✅ PYTHON flake8 206 0 2.51s
✅ PYTHON isort 206 0 0 0.99s
✅ PYTHON mypy 206 0 13.74s
✅ PYTHON pylint 206 0 16.15s
⚠️ PYTHON pyright 206 342 23.93s
✅ PYTHON ruff 206 0 0 0.96s
✅ REPOSITORY checkov yes no 38.85s
✅ REPOSITORY git_diff yes no 0.45s
⚠️ REPOSITORY grype yes 1 26.49s
✅ REPOSITORY secretlint yes no 12.1s
✅ REPOSITORY trivy yes no 38.06s
✅ REPOSITORY trivy-sbom yes no 15.1s
⚠️ REPOSITORY trufflehog yes 1 20.48s
✅ SPELL cspell 679 0 30.87s
❌ SPELL lychee 339 2 23.66s
✅ XML xmllint 3 0 0 0.47s
✅ YAML prettier 160 0 0 8.29s
✅ YAML v8r 102 0 153.1s
✅ YAML yamllint 161 0 1.96s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 27, 2023

@nvuillam do you know what problem I may have here:

filtered_files = utils.filter_files(

What causes it not to filter files that it should?

https://github.com/oxsecurity/megalinter/actions/runs/6997890867/job/19035355401?pr=3155#step:9:294

Thank you!

@nvuillam
Copy link
Member

@bdovaz maybe your updates in utils_test.py ?

image

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 27, 2023

Either you didn't understand me or I didn't explain well.

I know that the problem is in that file but what I mean is that I have changed that instead of using a custom code I use the method:

utils.filter_files

But there is some input that I am passing it wrong because it returns files that it should not as I have put in the second link.

As I imagine that you created that method that's why I ask you if you saw at a glance something wrong.

Thanks!

@nvuillam
Copy link
Member

@bdovaz I built it a long time ago, I'm afraid you'll need a breakpoint ^^

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 28, 2023

@nvuillam and how do I mount the setup locally? Because since Gitpod was added the instructions have changed and following these steps:

https://megalinter.io/latest/contributing/#42-desktop

I find that it tells me that it can't find the "redis" module and I can't run the tests as I could before in VS Code following these steps:

https://megalinter.io/latest/contributing/#execute-the-tests-locally-visual-studio-code

@echoix
Copy link
Collaborator

echoix commented Nov 28, 2023

I find that it tells me that it can't find the "redis" module and I can't run the tests as I could before in VS Code following these steps:

It's because it needs a version of python greater than 3.11.3, like 3.11.4. A package made a hotfix and conditionally imported a package a different way on older versions, but their fix doesn't work properly. But what python is available in the Ubuntu repositories is too old. I need to add the requirements for adding a PPA, and add the https://launchpad.net/~deadsnakes/+archive/ubuntu/ppa ppa then reinstall python3.11. But it never works on first try, and I need to fix a broken installation each time. So I'm not sure how it's supposed to be.

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 28, 2023

@echoix thank you very much, updating to the latest 3.11.x fixed it for me.

How do you know it was that? The error didn't say anything other than it couldn't find the "redis" module.

My experience with python and its tooling is quite limited 🤣

@echoix
Copy link
Collaborator

echoix commented Nov 28, 2023

I don't remember exactly, but I followed the imports and read the code, and searched on google in parallel. I saw the offending code. I found the specific issue on github the other month, but lost track of it.

@echoix
Copy link
Collaborator

echoix commented Nov 28, 2023

See aio-libs/async-timeout#229 and redis/redis-py#2659

@bdovaz
Copy link
Collaborator Author

bdovaz commented Nov 29, 2023

@nvuillam ready for you to review!

The builds that fail are because of the same thing:

image

Complaining about a file that evidently does not yet exist in the main branch.

@nvuillam nvuillam merged commit 75df660 into main Nov 29, 2023
127 of 131 checks passed
@nvuillam nvuillam deleted the features/roslynator branch November 29, 2023 21:25
@nvuillam
Copy link
Member

@bdovaz thanks for this great addition 😍😍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Roslynator
3 participants