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

[apex] Update jorje #4605

Merged
merged 6 commits into from Sep 28, 2023
Merged

[apex] Update jorje #4605

merged 6 commits into from Sep 28, 2023

Conversation

adangel
Copy link
Member

@adangel adangel commented Jun 24, 2023

Related issues

Note: If we are lucky, we get the regression report as a downloadable artifact from the github actions run.

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)

@adangel adangel added this to the 7.0.0 milestone Jun 24, 2023
@pmd-test
Copy link

pmd-test commented Jun 24, 2023

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.
Download full report as build artifact
Compared to master:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 2 violations, 0 errors and 0 configuration errors.
Full report

Generated by 🚫 Danger

@rsoesemann
Copy link
Member

@adangel I got us real technical support from the Salesforce Apex team. Not only to help with Jorje (I know we eventually (when?) will replace it with an Open Source parser from Google) but also with revamping Security rules that need to take into account all the new native Apex language features for Security.

Is there anything Daniel Ballinger @FishOfPrey from Salesforce can help with Jorje or elsewhere?

@FishOfPrey
Copy link

Hi @adangel,

I’m from the Apex product team, and we're very interested in understanding if there's anything we can help with to get this PR complete and merged. The new as user syntax is a big push we're making in terms of easy security best practices and we'd love to see this merged so we can help contribute improved rule(s) and test cases around this for PMD7 to align with our updated best practice suggestions.

@kfidelak94
Copy link

Hi @adangel - I lead our developer experience products team, including Apex, here at Salesforce - just piling on here to express our desire to get this PR merged to get support for User Mode - please let us know how we can help.

@kfidelak94
Copy link

Hi @adangel - just checking in again - anything we can do to help get this PR merged? We really want to be able to promote use of this and continue to contribute around this but need this change merged first. Thanks so much.

@adangel
Copy link
Member Author

adangel commented Aug 22, 2023

I'll try to work on it, but it is very low on my priority list.

One question, though: Does anybody know from where com.google.common.collect.ConcatenatedLists and such classes come from? They are in the apex jorje blob, but I couldn't find them in guava...

@anand13s
Copy link
Contributor

@adangel the com.google.common.collect.ConcatenatedLists come from apex-commons module, an internal extension of google guava to support apex concepts. It looks like your PR handles not deleting these folders already - please let us know if you have any other questions.

@rsoesemann
Copy link
Member

rsoesemann commented Sep 21, 2023

Hi @adangel or @oowekyala is there anything I or people from Salesforce can do to move this forward and get it into the next release? Updating Jorje to at least parse USER_MODE is a crucial step 1 of the other open changes.

Please let me know if there is anything I can do to take work or decision of your shoulders here.

I guess the reason why this is blocked for a while is a deeper technical issue than the two remaining "conflicts" on the PR, right?

@adangel
Copy link
Member Author

adangel commented Sep 22, 2023

I guess the reason why this is blocked for a while is a deeper technical issue than the two remaining "conflicts" on the PR, right?

There are basically two reasons: Time and Time.

And I did not get real feedback, whether a PMD version built with this PR branch actually works (not that I have asked for that).

The technical issue is Jorje in general, TBH. The experimental-apex-parser branch (#3766) looks promising, there are only some little problems left (see #4479 (comment)).

My current though is, that I'll merge this PR because of the test cases, but for the final release of PMD 7, Jorje will be gone.

@rsoesemann
Copy link
Member

rsoesemann commented Sep 23, 2023

Thanks @adangel. I understand that you needed help and didn't get the responses. Salesforce is willing to change that. Therefore my private request to help them become official Apex maintainers.

I also understand that you in the mid-term want to get rid of Jorje and use the new Google parser. They will fully support that. But I am very thankful and they will be that you at least for one release add the new Jorje that is able to parse USER_MODE.

FYI @jfeingold35 @johnbelosf @kfidelak94 @anand13s

@adangel
Copy link
Member Author

adangel commented Sep 24, 2023

I also understand that you in the mid-term want to get rid of Jorje and use the new Google parser. They will fully support that.

That's good to know.

But I am very thankful and they will be that you at least for one release add the new Jorje that is able to parse USER_MODE.

I actually didn't plan any more release candidates before final 7.0.0. But if it helps, I can spin a rc4 release next weekend (it would be beneficial to also include the java21 changes).

@johnbelosf
Copy link

@adangel @rsoesemann I wanted to express our gratitude for helping move this issue along. I am reaching out to our teams to provide further feedback.

@rsoesemann rsoesemann removed the request for review from oowekyala September 26, 2023 09:20
@adangel adangel merged commit c2b2a32 into pmd:master Sep 28, 2023
3 checks passed
@adangel adangel deleted the issue-3973-apex-jorje branch September 28, 2023 09:14
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.

[apex] Update parser to support new 'as user' keywords (User Mode for Database Operations)
7 participants