Skip to content

JS/Ruby/Python: rename RegExpTreeView.qll to ReDoSUtilSpecific.qll#8532

Merged
aibaars merged 2 commits intogithub:mainfrom
aibaars:regex-refactor-2
Mar 30, 2022
Merged

JS/Ruby/Python: rename RegExpTreeView.qll to ReDoSUtilSpecific.qll#8532
aibaars merged 2 commits intogithub:mainfrom
aibaars:regex-refactor-2

Conversation

@aibaars
Copy link
Contributor

@aibaars aibaars commented Mar 22, 2022

In #8489 (review) @nickrolfe pointed out that

having two different RegExpTreeView.qll files seems undesirable. Can they be merged?

There are two files named RegExpTreeView.qll in the Python and Ruby libraries. One of them contains language specific implementations of classes/predicates needed the the shared ReDoSUtil.qll module. As a result they cannot be merged, however, renaming that module to ReDoSUtilSpecific.qll avoids the name clash and also makes it clear that the module is just "implementation details".

@aibaars aibaars force-pushed the regex-refactor-2 branch 3 times, most recently from a74124b to 93c0b59 Compare March 24, 2022 10:37
@aibaars aibaars changed the title JS/Ruby/Python: move more code into ReDoSUtil.qll JS/Ruby/Python: rename RegExpTreeView.qll to ReDoSUtilSpecific.qll Mar 28, 2022
@aibaars aibaars marked this pull request as ready for review March 28, 2022 10:18
@aibaars aibaars requested review from a team as code owners March 28, 2022 10:18
Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the tests pass

Copy link
Contributor

@erik-krogh erik-krogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JS 👍

@aibaars aibaars added the no-change-note-required This PR does not need a change note label Mar 28, 2022
Copy link
Contributor

@nickrolfe nickrolfe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ruby 👍

@aibaars aibaars merged commit 031d183 into github:main Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants