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

New Rules: S5842, S5856, S6326 #6601

Closed
wants to merge 2 commits into from

Conversation

Corniel
Copy link
Contributor

@Corniel Corniel commented Jan 4, 2023

I implemented 3 regular expression related rules:

I combined them, as I was searching for a way how the different regex rules could reuse code to reduce the required plumbing. I investigated the (re)use of MS internal RegexTree. Unfortunately, that did not help much. The real showstopper in the end is that the RegexNode's in MS's regex tree, do not make a distinction between AAA and A{3}, or A+ and A{1,}. This makes that most analysis can not performed.

The regex helper types I introduced:

RegexTree

Allows to analyze a regex (in relation to its RegexOptions).

RegexNode

Contains the Regex pattern and Regex options related SyntaxNode's. Has factory methods to construct from

  • ObjectCreationExpressions (new Regex())
  • InvocationExpression (Regex.*)
  • Attribute ([System.ComponentModel.DataAnnotations.RegularExpression])

The generated regex attribute should also be taken into account. That is on my TODO.

The 3 rules I choose, do not require a (full) parsing of the regular expression itself. That would blown up this PR too much.

@pavel-mikula-sonarsource
Copy link
Contributor

Hi @Corniel,

this looks good from a first quick view on the overall structure.

Can you please split it into 3 PRs, one for each rule? You can keep this all together in a separate branch in your repo, so I have a reference for shared code and avoid complaining about unused stuff.

To give you more context on the Regex, there exist a Java-based engine to report more complex structures inside the regex itself. So please do not implement rules that would require more complex logic or sematnic understanding of the regex. These 3 looks simple (simple string based), so they are fine.

Thanks

@Corniel
Copy link
Contributor Author

Corniel commented Jan 16, 2023

@pavel-mikula-sonarsource I'll try to split them. I had a look at the Java code. As mentioned, on purpose, I choose 3 rules that did not need more then simple string based rules.

@pavel-mikula-sonarsource
Copy link
Contributor

Hi @Corniel,

Do you plan to work on the last rule S6326? That one should be easy to do with the scaffolding that is already in place.

@Corniel
Copy link
Contributor Author

Corniel commented Apr 19, 2023

@pavel-mikula-sonarsource Yes, I've plans to so.

@Corniel
Copy link
Contributor Author

Corniel commented Jun 7, 2023

@pavel-mikula-sonarsource See: #7345

@Corniel Corniel closed this Jun 7, 2023
@Corniel Corniel deleted the corniel/regex-rules branch June 7, 2023 06:55
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.

2 participants