Skip to content

[apex] Update jorje#4605

Merged
adangel merged 6 commits into
pmd:masterfrom
adangel:issue-3973-apex-jorje
Sep 28, 2023
Merged

[apex] Update jorje#4605
adangel merged 6 commits into
pmd:masterfrom
adangel:issue-3973-apex-jorje

Conversation

@adangel

@adangel adangel commented Jun 24, 2023

Copy link
Copy Markdown
Member

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
@adangel adangel force-pushed the issue-3973-apex-jorje branch from 6da4f35 to 8610c01 Compare June 24, 2023 19:35
@adangel adangel force-pushed the issue-3973-apex-jorje branch from 8610c01 to 56d5bfc Compare June 24, 2023 20:46
@ghost

ghost commented Jun 24, 2023

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.
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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
Copy Markdown

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

adangel commented Aug 22, 2023

Copy link
Copy Markdown
Member Author

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
Copy Markdown
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

rsoesemann commented Sep 21, 2023

Copy link
Copy Markdown
Member

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

adangel commented Sep 22, 2023

Copy link
Copy Markdown
Member Author

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

rsoesemann commented Sep 23, 2023

Copy link
Copy Markdown
Member

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

adangel commented Sep 24, 2023

Copy link
Copy Markdown
Member Author

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
Copy Markdown

@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
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)

6 participants