feat: generic parsing interface and parsing of JavaPP annotations #134

AlexanderSchultheiss · 2024-02-21T13:55:59Z

Description

The ArgoUML-SPL has been build using the Java Preprocessor library which mimics the conditional compilation of the C preprocessor. Due to JavaPP using a slightly different Syntax and being hidden behind line comments, we cannot simply apply our current parsing pipeline to ArgoUML.

This pull request pull up the necessary interfaced of our existing parsing pipeline to allow for different parser implementations. Additionally, it includes a parser for JavaPP.

Notes

I have integrated this version of DiffDetective in VEVOS and I was able to extract a ground truth for ArgoUML. However, ArgoUML only has one commit in which the variability is edited and this commit appears to contain files with missing 'endif' statements, which causes errors to be reported.

…change

…NodeType.java

…on in preparation for JPP parsing

AlexanderSchultheiss · 2024-02-21T13:58:10Z

@pmbittner, for some reason, GitHub is not able to determine that some files have simply been renamed or moved and instead shows them as fully deleted and re-added. This might be a bit confusing.

AlexanderSchultheiss · 2024-02-21T14:02:13Z

Have to fix Javadocs

AlexanderSchultheiss · 2024-02-21T14:14:45Z

It seems that the nix build breaks because I incremented DiffDetective's version number. How can I update the build files accordingly?

pmbittner

Hi @AlexanderSchultheiss ,

these changes look great. Thank you! 😃 The documentation is awesome, too.

I have one major comment: Should we really include the heavy reformatting changes? The reformatting of JavaDoc (e.g., to a certain line width limit) look good to me but many files also have their indentation changed extensively. Next time, I perform some changes in IntelliJ, all the formatting will be reverted, yielding large commits again.

Is this some setting in NeoVim? It might be very hard to change the indentation retroactively, so I can live with the changes. Maybe we should add a style configuration (.editorconfig) to DiffDetective or so.

src/main/java/org/variantsync/diffdetective/feature/AnnotationParser.java

src/main/java/org/variantsync/diffdetective/feature/DiffLineFormulaExtractor.java

src/main/java/org/variantsync/diffdetective/datasets/predefined/Marlin.java

src/main/java/org/variantsync/diffdetective/feature/PropositionalFormulaParser.java

src/main/java/org/variantsync/diffdetective/feature/cpp/CPPDiffLineFormulaExtractor.java

src/main/java/org/variantsync/diffdetective/feature/jpp/JPPDiffLineFormulaExtractor.java

src/main/java/org/variantsync/diffdetective/variation/NodeType.java

pmbittner · 2024-02-21T15:33:12Z

src/test/java/CPPParserTest.java

+                new TestCase("#ifdef \\U0001000", "__B_SLASH__U0001000"),
+                new TestCase("#if (defined(NAME) && (NAME >= 199905) && (NAME < 1991011)) ||     (NAME >= 300000) || defined(NAME)", "(DEFINED___LB__NAME__RB__&&(NAME__GEQ__199905)&&(NAME__LT__1991011))||(NAME__GEQ__300000)||DEFINED___LB__NAME__RB__"),
+                new TestCase("#if __has_warning(\"-Wa-warning\"_foo)",
+                        "__HAS_WARNING___LB____QUOTE____SUB__Wa__SUB__warning__QUOTE_____foo__RB__")
        );


Why is this reindentation necessary? Why should we use 8 spaces instead of 4 spaces now? (Next time I change something in IntelliJ, this styling will be reset and we have huge patches again. 😵 )

I have not used nvim because I was worried about my style being applied to reformat the entire codebase 🙈 . I only used IntelliJ which did this autoformatting.

Oh I see. Seems we have different settings then. I guess its time for a common .editorconfig then. I will open an issue soon then. You dont have to revert the indentation stuff. That is too cumbersome I guess.

Have you tried formatting he file with your Intellij? I am using the default settings, because I only just installed it on my new Laptop. Perhaps something else in the implementation had changed since the last time it was formatted?

I just formatted this block of TestCases in IntelliJ and the same formatting as you have was applied. Interesting...

pmbittner · 2024-02-21T15:38:30Z

sorry, hit the wrong button and closed by accident 😓

These were missed in 569f663.

ibbem · 2024-02-21T16:18:04Z

You missed the version bump in seven other files. Please use something like rg -F "2.1.0" or grep -F "2.1.0" -R to find all version numbers next time. You can also automatically change these places using rg -F "2.1.0" --files-with-matches | xargs sed -i 's/2\.1\.0/2.2.0/g' after verifying that the matches are actually all version numbers.

@pmbittner Should I create an issue for making these files independent of the version number such that we have one canonical place for changing the version number (e.g., pom.xml).

ibbem · 2024-02-21T16:25:33Z

I am also a NeoVim user. In case you have problems with your setup I might be able to help you.

AlexanderSchultheiss · 2024-02-21T18:09:38Z

You missed the version bump in seven other files. Please use something like rg -F "2.1.0" or grep -F "2.1.0" -R to find all version numbers next time. You can also automatically change these places using rg -F "2.1.0" --files-with-matches | xargs sed -i 's/2\.1\.0/2.2.0/g' after verifying that the matches are actually all version numbers.

@pmbittner Should I create an issue for making these files independent of the version number such that we have one canonical place for changing the version number (e.g., pom.xml).

Having to do this is not ideal. I searched for the version numbers, but I was unsure which ones I can change safely, because I am not familiar with nix and was not sure whether I would break stuff. I think that having only one place where the version number is specified would be great.

AlexanderSchultheiss · 2024-02-21T18:11:28Z

I am also a NeoVim user. In case you have problems with your setup I might be able to help you.

I have not configured jdtls to use Intellij's style for auto-formatting. How have you integrated Intellij's style in your configuration?

…ssorAnnotationParser

pmbittner · 2024-02-21T22:10:05Z

@pmbittner Should I create an issue for making these files independent of the version number such that we have one canonical place for changing the version number (e.g., pom.xml).

That is a good idea, given that also I missed this in my review (even though I incremented the version number correctly just two weeks ago).

…s upon implementation changes

ibbem · 2024-03-04T22:01:16Z

Sorry for the late answer, I totally forgot this.

I am also a NeoVim user. In case you have problems with your setup I might be able to help you.

I have not configured jdtls to use Intellij's style for auto-formatting. How have you integrated Intellij's style in your configuration?

No actually not. I will open an issue for creating an editor config or, if possible, start agreeing on a single formatter.

I also use jdtl, however I only format lines that I actually changed: The vim.lsp.buf.format action can also act on a visual selection, so I just select the code I edited and only reformat that particular chunk. It is probably possible to integrate this with Git somehow to only reformat changed lines but I haven't done this yet.

pmbittner · 2024-03-05T08:36:15Z

Hi all,

thanks for opening an issue @ibbem . We can proceed to merge this PR for now and deal with indentation and formatting later. Thanks @AlexanderSchultheiss for all the improvements. :)

pmbittner · 2024-03-05T08:44:26Z

src/test/java/CPPParserTest.java

+                new TestCase("#ifdef \\U0001000", "__B_SLASH__U0001000"),
+                new TestCase("#if (defined(NAME) && (NAME >= 199905) && (NAME < 1991011)) ||     (NAME >= 300000) || defined(NAME)", "(DEFINED___LB__NAME__RB__&&(NAME__GEQ__199905)&&(NAME__LT__1991011))||(NAME__GEQ__300000)||DEFINED___LB__NAME__RB__"),
+                new TestCase("#if __has_warning(\"-Wa-warning\"_foo)",
+                        "__HAS_WARNING___LB____QUOTE____SUB__Wa__SUB__warning__QUOTE_____foo__RB__")
        );


I just formatted this block of TestCases in IntelliJ and the same formatting as you have was applied. Interesting...

AlexanderSchultheiss added 18 commits February 19, 2024 19:45

refactor: introduce AnnotationParser interface to allow for parser ex…

359a009

…change

refactor: move C preprocessor-related implementations to own package

c3cba81

fix: specify the correct matcher group id

0fadf87

todos: add todo about AnnotationType to NodeType conversion

bc7747b

docs: short addendum to docstrings of AnnotationParser

e853ff7

refactor: move initialization of NodeType based on AnnotationType to …

90d19d0

…NodeType.java

tests: add first test definitions for JavaPP parsing

92d66c1

feat: add initial ANTLR grammar for JPP

2d98af2

refactor: change rule names of JPP grammar

def9aa7

refactor: pull up interface of CPP diff line extraction and abstracti…

f200c7a

…on in preparation for JPP parsing

refactor: reduce overhead of AnnotationParser implementations

5407946

feat: add stubs for JPP expression abstraction

5262889

refactor: prepare JPPParserTest for more test cases

a7a247d

docs: add comments describing the parser rules

cbbb3b4

feat: first working version of JavaPP parsing

d239664

test: add test for parsing entire JPP diff

6d9e315

chore: increment DiffDetective version

569f663

refactor: use the more general type

387c78f

AlexanderSchultheiss requested a review from pmbittner February 21, 2024 13:56

fix(docs): fix broken blocks in Docstring

f356365

pmbittner requested changes Feb 21, 2024

View reviewed changes

pmbittner closed this Feb 21, 2024

pmbittner reopened this Feb 21, 2024

pmbittner assigned AlexanderSchultheiss Feb 21, 2024

pmbittner added enhancement New feature or request Refactoring labels Feb 21, 2024

Update the version number in the remaining files

f91123b

These were missed in 569f663.

AlexanderSchultheiss added 5 commits February 21, 2024 19:33

docs: add docstring for AnnotationParser

f2d46b6

docs: update documentation of DiffLineFormulaExtractor

50c1d8a

refactor: let Marlin.java use a custom constructor method of Preproce…

16c6586

…ssorAnnotationParser

docs: revise JPPDiffLineFormulaExtractor docstring

987fe11

docs: add comment about Javadoc-related import

7a99762

AlexanderSchultheiss added 2 commits February 22, 2024 07:29

refactor: use switch expression

7433fa7

refactor: exchange default with actual variant to cause compile error…

e25fad4

…s upon implementation changes

ibbem mentioned this pull request Mar 4, 2024

Single source of truth for the DiffDetective version #135

Merged

pmbittner approved these changes Mar 5, 2024

View reviewed changes

pmbittner merged commit 6da27d2 into develop Mar 5, 2024
2 checks passed

pmbittner mentioned this pull request Apr 27, 2024

Release 2.2.0 #141

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: generic parsing interface and parsing of JavaPP annotations #134

feat: generic parsing interface and parsing of JavaPP annotations #134

AlexanderSchultheiss commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

pmbittner left a comment

pmbittner Feb 21, 2024

AlexanderSchultheiss Feb 21, 2024

pmbittner Feb 21, 2024

AlexanderSchultheiss Feb 22, 2024

pmbittner Mar 5, 2024

pmbittner commented Feb 21, 2024

ibbem commented Feb 21, 2024

ibbem commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

pmbittner commented Feb 21, 2024

ibbem commented Mar 4, 2024

pmbittner commented Mar 5, 2024

pmbittner Mar 5, 2024

feat: generic parsing interface and parsing of JavaPP annotations #134

feat: generic parsing interface and parsing of JavaPP annotations #134

Conversation

AlexanderSchultheiss commented Feb 21, 2024

Description

Notes

AlexanderSchultheiss commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

pmbittner left a comment

Choose a reason for hiding this comment

pmbittner Feb 21, 2024

Choose a reason for hiding this comment

AlexanderSchultheiss Feb 21, 2024

Choose a reason for hiding this comment

pmbittner Feb 21, 2024

Choose a reason for hiding this comment

AlexanderSchultheiss Feb 22, 2024

Choose a reason for hiding this comment

pmbittner Mar 5, 2024

Choose a reason for hiding this comment

pmbittner commented Feb 21, 2024

ibbem commented Feb 21, 2024

ibbem commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

AlexanderSchultheiss commented Feb 21, 2024

pmbittner commented Feb 21, 2024

ibbem commented Mar 4, 2024

pmbittner commented Mar 5, 2024

pmbittner Mar 5, 2024

Choose a reason for hiding this comment