Skip to content

[apex] Remove Jorje leaks outside ast package#4081

Merged
adangel merged 1 commit into
pmd:masterfrom
eklimo:remove-leaks
Aug 24, 2022
Merged

[apex] Remove Jorje leaks outside ast package#4081
adangel merged 1 commit into
pmd:masterfrom
eklimo:remove-leaks

Conversation

@eklimo

@eklimo eklimo commented Aug 5, 2022

Copy link
Copy Markdown
Contributor

Describe the PR

Remove some of the references to the internal Jorje parser outside the ast package.

Operators

  • Create ApexOperator class to wrap internal operator classes.
  • Refactor operator expression nodes to use ApexOperator.
  • Refactor metrics classes to use ApexOperator.

Tests

  • Remove references to internal Jorje nodes.

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@oowekyala oowekyala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this change! I'm happy to see this moving forward.

In the interest of minimizing divergences between master and your feature branch, I think all the changes to the test sources could be merged as is into master (we don't have compatibility guarantees there).

It would also be possible to introduce new methods (eg getOp) which use the new types, and deprecate the old getOperator methods instead of replacing them and breaking compatibility. If we do that then we could also merge this change into master.

Comment thread pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTestBase.java Outdated
Comment thread pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTestBase.java Outdated
@eklimo eklimo changed the base branch from experimental-apex-parser to master August 6, 2022 02:12
@eklimo eklimo requested a review from oowekyala August 6, 2022 02:15
@ghost

ghost commented Aug 6, 2022

Copy link
Copy Markdown
1 Message
📖 Compared to master:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.
Full report

Generated by 🚫 Danger

Comment thread pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ApexOperator.java Outdated
@eklimo eklimo requested a review from oowekyala August 6, 2022 22:34
Comment thread pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/UnaryOperator.java Outdated
Comment thread pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/BinaryOperator.java Outdated
@eklimo eklimo requested a review from oowekyala August 10, 2022 07:16
@oowekyala oowekyala changed the title Remove Jorje leaks outside ast package [apex] Remove Jorje leaks outside ast package Aug 10, 2022
Operators
- Create operator enums to wrap internal Jorje operators.
- Create getters returning new operator enums in operator-expression
  nodes. Deprecate old getters.
- Refactor metrics classes to use new operator enums.

Tests
- Remove references to internal Jorje nodes.

@oowekyala oowekyala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the changes!

@oowekyala oowekyala self-assigned this Aug 11, 2022

@adangel adangel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@adangel adangel added this to the 6.49.0 milestone Aug 24, 2022
@adangel adangel merged commit b9eec54 into pmd:master Aug 24, 2022
adangel added a commit to adangel/pmd that referenced this pull request Aug 24, 2022
adangel added a commit to adangel/pmd that referenced this pull request Aug 24, 2022
[apex] Remove Jorje leaks outside ast package pmd#4081
@eklimo eklimo deleted the remove-leaks branch August 24, 2022 17:06
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