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 parser to support new 'as user' keywords (User Mode for Database Operations) #3973

Closed
Marcono1234 opened this issue May 14, 2022 · 6 comments · Fixed by #4605
Closed
Labels
a:bug PMD crashes or fails to analyse a file.
Milestone

Comments

@Marcono1234
Copy link

Affects PMD Version:

Commit 9d23d79 (PMD 6.45.0)

Description:

It looks like the Maven dependencies declared by pmd-apex-jorje do not match the actual dependencies of the currently used apex-jorje-lsp.jar anymore. In particular jol-core seems to not be used anymore, and some other dependencies are pulled in as transitive dependencies and might not have to be declared explicitly.

Do you think it would be possible to adjust the create-local-repo.sh script to automatically adjust the pmd-apex-jorje POM with the information from the META-INF/maven directory in the apex-jorje-lsp.jar file? (with exclusions for the com.salesforce.apex artifacts, since they are part of the JAR)

Maybe it would also be good to add a small comment to create-local-repo.sh explaining why com/google/common* is not removed from the JAR. Currently one has to dig into the Git history to find #773 (comment) which mentions that apex-jorje-lsp.jar is using a modified version of Guava.

@Marcono1234 Marcono1234 added the a:bug PMD crashes or fails to analyse a file. label May 14, 2022
@adangel
Copy link
Member

adangel commented Jun 24, 2023

Do you think it would be possible to adjust the create-local-repo.sh script to automatically adjust the pmd-apex-jorje POM with the information from the META-INF/maven directory in the apex-jorje-lsp.jar file? (with exclusions for the com.salesforce.apex artifacts, since they are part of the JAR)

No, because it is incomplete. There is also DEPENDENCIES and probably more, which is just inside the jar. No idea, how this blob is created, since it is not open source.

We are anyway trying to get rid of jorje in order to use a true open source alternative - see #3766.

I'm updating now and I found out, that there are additional classes in com.google.common which are not available anywhere else (e.g. com.google.common.collect.MoreMaps). I'll update the script to only keep the differing files (one manual comparison).

@adangel adangel added this to the 7.0.0 milestone Jun 24, 2023
@adangel adangel changed the title [apex] pmd-apex-jorje dependencies are outdated [apex] Update parser to support new 'as user' keywords (User Mode for Database Operations) Jun 24, 2023
adangel added a commit to adangel/pmd that referenced this issue Jun 24, 2023
@adangel adangel mentioned this issue Jun 24, 2023
4 tasks
adangel added a commit to adangel/pmd that referenced this issue Jun 24, 2023
@Marcono1234
Copy link
Author

Marcono1234 commented Jun 24, 2023

We are anyway trying to get rid of jorje in order to use a true open source alternative - see #3766.

That sounds promising, thanks for the info!

I'm updating now and I found out, that there are additional classes in com.google.common which are not available anywhere else (e.g. com.google.common.collect.MoreMaps). I'll update the script to only keep the differing files (one manual comparison).

Even with the current Jorje setup under certain circumstances you could already run into issues when some Guava classes were loaded from the Jorje JAR but other classes had already been loaded by a parent class loader from a regular Guava JAR you had on your classpath. If the Jorje Guava classes tried to access the regular ones in the same package but from the regular Guava JAR, an IllegalAccessError could occur, see https://stackoverflow.com/q/7076414 which is about that problem in general.

I am a bit afraid that your proposed changes of removing some of the Guava classes could make it more likely that such issues occur.

Maybe an alternative would be to change the package name of the Guava classes in Jorje to avoid conflicts with regular Guava versions, for examples with the Jar Jar Links plugin or similar. Though of course changing package names is a bit risky in case there is reflective usage which is not detected by the tool performing the renaming.

@rsoesemann
Copy link
Member

Screenshot 2023-09-30 at 22 51 56 Strange I still see the same parsing issue ?

@adangel
Copy link
Member

adangel commented Oct 1, 2023

It works with PMD 7.0.0-rc4:

public class UserMode {
  public void coverAllCasesWithTest() {
    Contact c;
    c = [SELECT Name FROM Contact WITH USER_MODE];
    // Should be flagged a critical issue
    Contact c2;
    insert c2;
    // Should be ignored
    insert as user c2;
    // Should be at best a warning because it ignores CRUD but explicitly
    insert as system c2;
    // Should be ignored
    update as user c2;
  }
}

grafik

Are you sure you are using rc4? From which software is your screenshot?

@pablogomez2197
Copy link

pablogomez2197 commented Apr 4, 2024

Hello we have installed the version [v7.0.0.rc4] and we have the error. When we are working on a apex class some pmd error appears but when we put 'as user' all that errors dissapears without any reason. Here are the pictures:
image
and with the 'as user':
image
I don't know if is a problem of the version or something else. I think these is the same problem of these issue.

Thanks in advance.

@jsotuyod
Copy link
Member

jsotuyod commented Apr 4, 2024

PMD 7.0.0 final release is out, I recommend you try it out and open a new issue if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:bug PMD crashes or fails to analyse a file.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants