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

Complex Cases #21

Open
2 tasks
pouryafard75 opened this issue Jan 13, 2023 · 6 comments
Open
2 tasks

Complex Cases #21

pouryafard75 opened this issue Jan 13, 2023 · 6 comments
Labels
help wanted Extra attention is needed

Comments

@pouryafard75
Copy link
Owner

pouryafard75 commented Jan 13, 2023

Dir: community/server/src/main/java/org/neo4j/server/security/

ssl/SslCertificateFactory.java ssl/Certificates.java

MultiMappings must be discussed. @tsantalis

  • Case 2: JetBrains/intellij-community@04397f4
    There is a recursive method checkForTestRoots. @tsantalis Should we match
    Line 139 parent commit testFolders.addAll(ModuleRootManager.getInstance(srcModule).getSourceRoots(JavaSourceRootType.TEST_SOURCE));
    with Line 140 child commit testFolders.addAll(ModuleRootManager.getInstance(module).getSourceRoots(JavaSourceRootType.TEST_SOURCE));?

Line 139 parent is already matched with Line 132 child.
image

@tsantalis
Copy link

@pouryafard75
What is the issue here?

@pouryafard75
Copy link
Owner Author

pouryafard75 commented Jan 31, 2023

@tsantalis
For the case 1:
Inline Method Refactoring private ensureFolderExists(path File) : void is reported and the bodyMapper is also correct but the statement containing method invocation call is not matched. In other words, the following statements can also be mapped:

  • Line 92 parent => Line 74 child
  • Line 93 parent => Line 75 child

@tsantalis
Copy link

ensureFolderExists(certificatePath.getParentFile());
ensureFolderExists(privateKeyPath.getParentFile());

should match with

certificatePath.getParentFile().mkdirs();
privateKeyPath.getParentFile().mkdirs();

@tsantalis
Copy link

@pouryafard75
In case of duplicate mappings, as in case 1
we have a rule to discard the mappings involving a call to extracted or inlined method.

So, in this case the mappings shown in #21 (comment) will be discarded.
However, what we need is a sub-expression mapping between the expression passed as argument, which then becomes the expression used to call mkdirs()

We can generate these sub-expression mappings directly in the Extract/Inline Method refactoring objects,
without making mappings at statement level.

tsantalis added a commit to tsantalis/RefactoringMiner that referenced this issue Feb 9, 2023
@pouryafard75 pouryafard75 added the help wanted Extra attention is needed label Feb 14, 2023
@pouryafard75
Copy link
Owner Author

pouryafard75 commented Mar 12, 2023

@tsantalis For case 2, the mappings are correct in my opinion. Let me know if you disagree.
I will add it to the test cases.

@tsantalis
Copy link

@pouryafard75
Case2 : L139 -> R132 is an exact match. Thus it should have higher priority than the inexact match L139 -> R140
The diff is correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants