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

enhancement: C# and VB .NET Format Fixes #3741

Merged
merged 9 commits into from
Jul 21, 2024

Conversation

TommyE123
Copy link
Contributor

C# and VB .NET Format Fixes

Issues Identified

  1. Performance: Scanning operations often take hours or timeout instead of completing within minutes.
    a. Redundant scanning: Both C# and VB linters scan for both file types if either exists, leading to duplicate scans and unnecessary processing.
    b. Overzealous linting: The dotnet format linter runs the command for every .cs and .vb file, causing thousands of duplicate logs and severely impacting performance.

  2. Inaccurate Error Count: The summary results and logs show incorrect error amounts.

  3. Multiple Solution Files: The linter fails to run if there's more than one .sln file in the root directory.

  4. Subdirectory Solution Files: Neither language can run if the .sln file is in a subdirectory.

  5. Project File Specification: Unable to specify a list of .csproj or .vbproj files to lint.

Note: While some of these issues might be circumvented using configuration overrides, this approach would require in-depth knowledge of MegaLinter and how to achieve better outcomes. The proposed changes aim to address these issues more systematically.

Proposed Changes

  1. Performance Improvement:

    • Changed linters to use cli_lint_mode: project instead of list_of_files.
      • Pro: Massive speed improvement.
      • Con: Displays N/A for files linted (minor drawback).
    • Added language-specific file inclusion:
      • C#: --include **/*.cs
      • VB: --include **/*.vb
      • Reduces issue duplication and prevents cross-language linting errors.
  2. Error Count Fix: Implemented correct regex to accurately count errors across all files for each language.

  3. Solution File Handling:

    • Removed {{WORKSPACE}} argument to allow specifying a single .sln file.
    • Addresses issues with multiple .sln files and subdirectory locations.
  4. Command Structure:

    • Moved --verify-no-changes and new include arguments to extra args.
    • Allows for overriding .sln or .proj files in the correct command position.

These changes aim to significantly improve performance, accuracy, and flexibility of the dotnet format linter for both C# and VB.NET projects, without requiring extensive MegaLinter configuration knowledge.

@echoix
Copy link
Collaborator

echoix commented Jul 4, 2024

Do you mean the tool when called on a workspace gets launched multiple times?

@echoix
Copy link
Collaborator

echoix commented Jul 4, 2024

I took a look, but I don't know what to think very much about it.

Does dotnet format run on files that are more in their web-based environment (not exactly aspnet, but the ones where there is html and csharp)? If so, is using the glob limited to .cs prevent users from configuring it?

@TommyE123
Copy link
Contributor Author

I've attached the results showing the before and after effects of the changes I've made. This is a basic test solution with multiple projects and minimal code.

As you can see, the current MegaLinter version duplicates the complete error list for every file, significantly increasing the processing time as it's running the command for each file found, which isn't necessary. For more complex solutions, this approach could take hours to complete, potentially timing out before finishing.

The revised changes eliminate these repetitive error listings, improving efficiency.
Additionally, I've resolved an issue where the C# linter was processing VB files and vice versa. Now, each linter only processes its respective language files.

Regarding file types, I couldn't find any evidence through searching or my testing to suggest that the dotnet format command would process file types other than .cs and .vb, which I'm setting the include path to find for each.

Current logs.txt
New logs.txt

As always, if there are any issues or concerns, please let me know.
Tom

@TommyE123 TommyE123 marked this pull request as ready for review July 21, 2024 17:15
@nvuillam
Copy link
Member

Looks great to me, but as I have no experience about vb or dotnet, I prefer to let @echoix or @bdovaz validate :)

@nvuillam nvuillam merged commit 39b597d into oxsecurity:main Jul 21, 2024
6 checks passed
@nvuillam
Copy link
Member

thanks @echoix :)

& thanks @TommyE123 for the PR :)

szinn referenced this pull request in szinn/k8s-homelab Aug 20, 2024
… ) (#4161)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [oxsecurity/megalinter](https://togithub.com/oxsecurity/megalinter) |
action | major | `v7.13.0` -> `v8.0.0` |

---

### Release Notes

<details>
<summary>oxsecurity/megalinter (oxsecurity/megalinter)</summary>

###
[`v8.0.0`](https://togithub.com/oxsecurity/megalinter/blob/HEAD/CHANGELOG.md#v800---2024-08-19)

[Compare
Source](https://togithub.com/oxsecurity/megalinter/compare/v7.13.0...v8.0.0)

-   Reporters
- New
[**ApiReporter**](https://megalinter.io/beta/reporters/ApiReporter/)
(can be used to build Grafana dashboards), by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3540](https://togithub.com/oxsecurity/megalinter/pull/3540)

[![Upgrade to v8
Video](https://img.youtube.com/vi/vbx-ifa1oXE/0.jpg)](https://www.youtube.com/watch?v=vbx-ifa1oXE)

- Removed deprecated linters, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3854](https://togithub.com/oxsecurity/megalinter/pull/3854)
- CSS_SCSSLINT: [Project discontinued and advising to use
stylelint](https://togithub.com/sds/scss-lint#notice-consider-other-tools-before-adopting-scss-lint)
- OPENAPI_SPECTRAL: Replaced by
[API_SPECTRAL](https://megalinter.io/latest/descriptors/api_spectral/)
(same linter but more formats handled)
- SQL_SQL_LINT: [Project no longer
maintained](https://togithub.com/joereynolds/sql-lint/issues/262)

-   Core
- Hide to linters by default all environment variables that contain
**TOKEN**, **USERNAME** or **PASSWORD**, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3881](https://togithub.com/oxsecurity/megalinter/pull/3881)
- Allow to override CLI_LINT_MODE when defined as project, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3772](https://togithub.com/oxsecurity/megalinter/pull/3772)
- Allow to use absolute paths for LINTER_RULES_PATH, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3775](https://togithub.com/oxsecurity/megalinter/pull/3775)
- Allow to update variables from [PRE/POST
Commands](https://megalinter.io/latest/config-precommands/) using
`output_variables` property, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3861](https://togithub.com/oxsecurity/megalinter/pull/3861)

-   Media
- [MegaLinter: un linter pour les gouverner
tous](https://blog.wescale.fr/megalinter-un-linter-pour-les-gouverner-tous)
(FR), by [Guillaume
Arnaud](https://www.linkedin.com/in/guillaume-arnaud/) from
[WeScale](https://www.wescale.fr/)
-
[MegaLinter](https://blog.stephane-robert.info/docs/developper/autres-outils/linters/megalinter/),
by [Stéphane Robert](https://www.linkedin.com/in/stephanerobert1/), from
[3DS OutScale](https://fr.outscale.com/)
- [30 Seconds to Setup MegaLinter: Your Go-To Tool for Automated Code
Quality](https://medium.com/@&#8203;caodanju/30-seconds-to-setup-megalinter-your-go-to-tool-for-automated-code-quality-and-iac-security-969d90a5a99c),
by [Peng Cao](https://www.linkedin.com/in/peng-cao-83b6a2103/) |

-   Linters enhancements
- [bandit](https://megalinter.io/latest/descriptors/python_bandit/) Call
bandit with quiet mode to generate less logs, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3892](https://togithub.com/oxsecurity/megalinter/pull/3892)
- [grype](https://megalinter.io/latest/descriptors/repository_grype/)
Count number of errors returned by Grype, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3906](https://togithub.com/oxsecurity/megalinter/pull/3906)
- [yamllint](https://megalinter.io/latest/descriptors/yaml_yamllint) Fix
yamllint default format to avoid special characters or GitHub sections
in text logs, by [@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3898](https://togithub.com/oxsecurity/megalinter/pull/3898)

-   Fixes
- [terrascan](https://runterrascan.io/) fixed errors and removed
redundant code, by [@&#8203;TommyE123](https://togithub.com/TommyE123)
in
[https://github.com/oxsecurity/megalinter/pull/3767](https://togithub.com/oxsecurity/megalinter/pull/3767)
-
[dotnet-format](https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-format)
various performance improvements and ability to specify sln or proj
paths, by [@&#8203;TommyE123](https://togithub.com/TommyE123) in
[https://github.com/oxsecurity/megalinter/pull/3741](https://togithub.com/oxsecurity/megalinter/pull/3741)
- [swiftlint](https://togithub.com/realm/SwiftLint) Remove deprecated
argument --path
- Salesforce linters: Disable SF CLI auto update warning, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3883](https://togithub.com/oxsecurity/megalinter/pull/3883)

-   Doc
- Add images and links to Git, CI/CD & other tools integrations at the
beginning of the README, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3885](https://togithub.com/oxsecurity/megalinter/pull/3885)
- Create README animated GIF presentation of MegaLinter, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3910](https://togithub.com/oxsecurity/megalinter/pull/3910)
- Format mkdocs search index in place, by
[@&#8203;echoix](https://togithub.com/echoix) in
[https://github.com/oxsecurity/megalinter/pull/3890](https://togithub.com/oxsecurity/megalinter/pull/3890)
- Use consistent spelling of 'flavor', by
[@&#8203;InputUsername](https://togithub.com/InputUsername) in
[https://github.com/oxsecurity/megalinter/pull/3789](https://togithub.com/oxsecurity/megalinter/pull/3789)

-   CI
- Fix docker warnings, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3853](https://togithub.com/oxsecurity/megalinter/pull/3853)
        -   FromAsCasing: 'as' and 'FROM' keywords' casing do not match
        -   NoEmptyContinuation: Empty continuation line
- SecretsUsedInArgOrEnv: Do not use ARG or ENV instructions for
sensitive data
- Port Beta workflows to use docker/metadata-action, by
[@&#8203;echoix](https://togithub.com/echoix) in
[https://github.com/oxsecurity/megalinter/pull/3860](https://togithub.com/oxsecurity/megalinter/pull/3860)
- AutoUpdate linters: Always create a PR if the job has been started
manually, by [@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3863](https://togithub.com/oxsecurity/megalinter/pull/3863)
- Add `skip_checkout: true` to default MegaLinter GitHub Action template
- Remove path filters in deploy-DEV workflow as it is a required check
by [@&#8203;echoix](https://togithub.com/echoix) in
[https://github.com/oxsecurity/megalinter/pull/3894](https://togithub.com/oxsecurity/megalinter/pull/3894)

-   mega-linter-runner
- Add new rules to upgrade to MegaLinter v8, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3896](https://togithub.com/oxsecurity/megalinter/pull/3896)
- Replace glob-promise by glob library, by
[@&#8203;nvuillam](https://togithub.com/nvuillam) in
[https://github.com/oxsecurity/megalinter/pull/3902](https://togithub.com/oxsecurity/megalinter/pull/3902)
        -   **Minimum NodeJs version is now 20.x**

-   Linter versions upgrades
- [ansible-lint](https://ansible-lint.readthedocs.io/) from 24.6.1 to
**24.7.0**
-
[bicep_linter](https://learn.microsoft.com/en-us/azure/azure-resource-manager/bicep/linter)
from 0.28.1 to **0.29.47**
- [black](https://black.readthedocs.io/en/stable/) from 24.4.2 to
**24.8.0**
- [cfn-lint](https://togithub.com/aws-cloudformation/cfn-lint) from
1.5.0 to **1.10.3**
    -   [checkov](https://www.checkov.io/) from 3.2.174 to **3.2.232**
- [clippy](https://togithub.com/rust-lang/rust-clippy) from 0.1.79 to
**0.1.80**
- [clj-kondo](https://togithub.com/borkdude/clj-kondo) from 2024.05.24
to **2024.08.01**
    -   [csharpier](https://csharpier.com/) from 0.28.2 to **0.29.0**
-
[cspell](https://togithub.com/streetsidesoftware/cspell/tree/master/packages/cspell)
from 8.10.4 to **8.14.1**
-
[dotnet-format](https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-format)
from 8.0.106 to **8.0.108**
    -   [flake8](https://flake8.pycqa.org) from 7.1.0 to **7.1.1**
- [golangci-lint](https://golangci-lint.run/) from 1.59.1 to **1.60.1**
- [grype](https://togithub.com/anchore/grype) from 0.79.2 to **0.79.5**
- [jsonlint](https://togithub.com/prantlf/jsonlint) from 14.0.3 to
**16.0.0**
    -   [kics](https://www.kics.io) from 2.1.1 to **2.1.2**
- [kubeconform](https://togithub.com/yannh/kubeconform) from 0.6.6 to
**0.6.7**
- [lightning-flow-scanner](https://togithub.com/Lightning-Flow-Scanner)
from 2.28.0 to **2.33.0**
- [mypy](https://mypy.readthedocs.io/en/stable/) from 1.10.1 to
**1.11.1**
- [php-cs-fixer](https://cs.symfony.com/) from 3.59.3 to **3.62.0**
- [phpcs](https://togithub.com/PHPCSStandards/PHP_CodeSniffer) from
3.10.1 to **3.10.2**
    -   [phpstan](https://phpstan.org/) from 1.11.9 to **1.11.11**
    -   [pmd](https://pmd.github.io/) from 7.3.0 to **7.4.0**
    -   [prettier](https://prettier.io/) from 3.3.2 to **3.3.3**
- [protolint](https://togithub.com/yoheimuta/protolint) from 0.50.2 to
**0.50.5**
    -   [pylint](https://pylint.readthedocs.io) from 3.2.5 to **3.2.6**
- [pyright](https://togithub.com/Microsoft/pyright) from 1.1.370 to
**1.1.376**
    -   [revive](https://revive.run/) from 1.3.7 to **1.3.9**
- [rstcheck](https://togithub.com/myint/rstcheck) from 6.2.1 to
**6.2.4**
    -   [rubocop](https://rubocop.org/) from 1.64.1 to **1.65.1**
- [ruff](https://togithub.com/astral-sh/ruff) from 0.5.1 to **0.6.1**
- [sfdx-scanner-apex](https://forcedotcom.github.io/sfdx-scanner/) from
4.3.2 to **4.4.0**
- [sfdx-scanner-aura](https://forcedotcom.github.io/sfdx-scanner/) from
4.3.2 to **4.4.0**
- [sfdx-scanner-lwc](https://forcedotcom.github.io/sfdx-scanner/) from
4.3.2 to **4.4.0**
- [snakemake](https://snakemake.readthedocs.io/en/stable/) from 8.15.2
to **8.18.1**
    -   [stylelint](https://stylelint.io) from 16.6.1 to **16.8.2**
- [swiftlint](https://togithub.com/realm/SwiftLint) from 0.55.1 to
**0.56.1**
- [syft](https://togithub.com/anchore/syft) from 1.8.0 to **1.11.0**
-
[terraform-fmt](https://developer.hashicorp.com/terraform/cli/commands/fmt)
from 1.9.0 to **1.9.4**
- [terragrunt](https://terragrunt.gruntwork.io) from 0.59.6 to
**0.66.8**
- [tflint](https://togithub.com/terraform-linters/tflint) from 0.52.0 to
**0.53.0**
- [trivy-sbom](https://aquasecurity.github.io/trivy/) from 0.53.0 to
**0.54.1**
- [trivy](https://aquasecurity.github.io/trivy/) from 0.53.0 to
**0.54.1**
- [trufflehog](https://togithub.com/trufflesecurity/trufflehog) from
3.79.0 to **3.81.9**
    -   [v8r](https://togithub.com/chris48s/v8r) from 3.1.0 to **4.0.1**
    -   [vale](https://vale.sh/) from 3.6.0 to **3.7.0**

</details>

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC40Mi4wIiwidXBkYXRlZEluVmVyIjoiMzguNDIuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUvZ2l0aHViLWFjdGlvbiIsInJlbm92YXRlL2dpdGh1Yi1yZWxlYXNlIiwidHlwZS9tYWpvciJdfQ==-->

Co-authored-by: repo-jeeves[bot] <106431701+repo-jeeves[bot]@users.noreply.github.com>
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.

3 participants