feat: CtCompilationUnit extends CtElement #2702

pvojtechovsky · 2018-10-20T18:15:47Z

The refactoring of compilation unit based on discussions in #2254 and #2701

There are several changes

C1) CompilationUnit now extends new CtCompilationUnit
C2) CtCompilationUnit extends CtElement
C3) CtCompilationUnit may have list of type references. before it had list of types.
C4) CtCompilationUnit has one CtPackageDeclaration, which references package.
C5) CtCompilationUnit may have module reference, before it had module.
C6) CtScanner visits CtCompilationUnit and it's package declaration, imports, types references or module reference.

surli · 2018-10-21T12:36:34Z

Why exactly this new element contains references for types? The type were belonging to CompilationUnit because in the reality the types are declared in their CU. Not sure to understand the move, especially if you keep an inheritance between CompilationUnit and CtCompilationUnit. (Not sure either why to keep the old CompilationUnit class, though except maybe for backward compatibility)

pvojtechovsky · 2018-10-21T16:08:19Z

an inheritance between CompilationUnit and CtCompilationUnit.

it is here only for backward compatibility. I will gladly remove old Compilation unit if @surli and @monperrus agrees that.

Why exactly CtCompilationUnit element contains references for types? The type were belonging to CompilationUnit because in the reality the types are declared in their CU.

The CtTypes belong to
A) CtPackage
B) CtCompilationUnit

But spoon model is a Tree by definition, so it is not allowed for CtType to have two parents.

First I started to implement CtCompilationUnit by way it contained CtType directly - saying it is OK that parent/child relationship is not correct in that case. But there were several problems

model consistency the model checking tests failed - OK they can be modified
removing of type from CtCompilationUnit did not worked
CtScanner.visitCompilationUnit(cu) visited CtType and all it's children twice. 1st because it contains declared CtPackage which contains CtType, and second because CtCompilationUnit contains declared types too.
cloning of CtCompilationUnit cloned whole subtree of CtPackage (including CtType) and whole sub tree of CtType. I as a client would expect that only CtCompilation unit is cloned and reused the existint CtType, but instead it cloned each CtType twice.

So then I found that it is much easier and correct too, when CtCompilationUnit has reference to CtPackage and reference to CtType. All the problems above are solved by that change.
And there is still method CtCompilationUnit.getDeclaredTypes, which is derived now and looks for CtTypes using internal references.

As a next step we can even add a collection of CtCompilationUnits somewhere into CtModel or into CtPackage and then CtCompilationUnit becomes normal member of spoon model. But that is again possible only if CtCompilationUnit contains references to types and package.

WDYT?

surli · 2018-10-22T09:41:31Z

So trying to understand the new hierarchy of Spoon concepts with your change. You'll have something like:

CtModel
|
 -- CtRootPackage
   |
    -- CtPackage
       |
        -- CtCompilationUnit (package declaration)
       |
        -- CtType
          |
           -- CtCompilationUnit (type declaration)

am I right? Isn't it a bit odd that it's the contrary of the reality: the compilation units actually precedes the declaration of package/types.

pvojtechovsky · 2018-10-22T10:41:57Z

This PR doesn't puts CtCompilationUnit into any parent. It is actually without parent.
But in future we should put CtCompilationUnit somewhere, so it makes sense to discuss it.

The compilation units actually precedes the declaration of package/types.

This is philosophical discussion ... and might be long. I do not want to attend ;-)

Do you have an alternative? Where CtCompilationUnit belongs to from your point of view? ... and I suggest to keep Spoon the model a "tree" do not switch it to something else ... it would cause many deep changes.

surli · 2018-10-22T11:32:27Z

The way I see it, the package contains a list of CtCompilationUnit, which themselves contains the CtType. The package only contains CtTypeReference.

Now if a CtPackage is declared in a compilation unit (package-info.java) then its parent is a CtCompilationUnit.
So we have something like

CtModel
|
--- CtRootPackage
 |
  --- CtCompilationUnit
    |
     --- CtPackage
       |
        --- CtCompilationUnit
          |
           --- CtType
       |
        --- CtCompilationUnit
          |
           --- CtType
       |
        --- CtPackage
           |
            --- CtCompilationUnit
               |
                --- CtType
               |
                --- CtType

pvojtechovsky · 2018-10-22T11:46:34Z

We speak about two models:

java runtime model - it doesn't contain CompilationUnit at all. Package contains Types. That model is original model of spoon and is good for analyzing and refactoring
java file system model - it maps types to CompilationUnits, which represents files in file system. That model is needed when spoon tries to print sources back into file system.

The java runtime model is primary target of Spoon library. So current Spoon model design Package has Types is correct from that point of view.
The java file system model is needed too. I understand it is mapping of primary (runtime) model to file system.

The java file system model is kind of extension of primary model. Therefore usage of type/package references to primary model makes sense.

surli · 2018-10-22T13:50:51Z

I'm really mixed on this one.
I don't really agree with you when you say:

The java runtime model is primary target of Spoon library.

I agree that the current model does not support directly the filesystem concepts (even though they can be retrieved using the API).
Now I think we both agree we cannot maintain two different models in Spoon, we want only one. If we modify the existing I would really prefer something closer to the source code, as it's the primary target of Spoon, so a model which would contain the concepts of my Java files.

The rationale behind that is:

if we break something, let's break it in something we would want to keep and maintain on the long term view
keep the model close to the concepts that are meaningful for Java devs, not something that's easy for us to develop

pvojtechovsky · 2018-10-22T17:53:30Z

@surli thank you for that open and constructive discussion. It is really helpful 👍

Now I think we both agree we cannot maintain two different models in Spoon, we want only one.

I agree.

if we break something, let's break it in something we would want to keep and maintain on the long term view

I agree with this sentence. But this sentence is not fitting to this PR, because this PR actually breaks nothing. All tests passed without need for bigger change.

The idea to have CtType contained in CtCompilationUnit is breaking. I do not think it is helpful and needed.

keep the model close to the concepts that are meaningful for Java devs, not something that's easy for us to develop

I agree with this sentence too :-)

But I think that CtCompilationUnit is secondary think, which should not bother Spoon clients who don't need it. Therefore I would prefer to have CtCompilationUnit somehow outside of primary spoon model.

Why I think it is not needed?

I am using java for about 17 years. And I never needed to think about compilation unit.
I am actively using Spoon for more then 1 year. And I had to learn what is compilation unit and how it works only when I was fixing internal spoon algorithms:

the fixing of compilation process - here I touched compilation unit first time in my live
the introducing CtCompilationUnit into model (this PR) to be able to finish sniper printer.

Now I understand what compilation unit is and I still think that I will not need it for next years of work with java and Spoon.
I can imagine only one use case where compilation unit is relevant for spoon client - when s/he writes refactoring which moves types from one file to another. I guess it will be less then 1% of spoon clients. Actually 0%.

I only need to know that about CompilationUntis: Spoon should keep by default the types in origin files (compilation units). I really do not want to touch them all the time I am processing the spoon model.

So the compilation units are secondary stuff which should be hidden somewhere in background ... but of course well designed and well accessible when somebody really needs it.

May be there is some misunderstanding ... in one of previous comments you draw this picture - as your understanding of what I probably plan to do:

CtModel
|
 -- CtRootPackage
   |
    -- CtPackage
       |
        -- CtCompilationUnit (package declaration)
       |
        -- CtType
          |
           -- CtCompilationUnit (type declaration)

but it is wrong. It doesn't fit to this PR or concepts I would like to have in Spoon.

I suggest one of these models

M1) compilation units in packages, because packages represents file system directories

CtModel
|
 -- CtRootPackage
   |
    -- CtPackage P1
       |
        -- CtCompilationUnit (package declaration which refers P1)
       |
        -- CtCompilationUnit (refers T1)
       |
        -- CtType T1

... or one of following models, because compilation units are not fitting to java runtime model at all.

M2) compilation units in root of model (e.g. Set). It would be very similar to what we already have in Spoon 7.1

CtModel
|
 -- CtRootPackage
|  |
|   -- CtPackage P1
|      |
|       -- CtType T1
 -- CtCompilationUnit (package declaration which refers P1)
|
 -- CtCompilationUnit (refers T1)

M3) compilation units in special model. Just an little alternative of M2.

CtModel
|
 -- CtRootPackage
|  |
|   -- CtPackage P1
|      |
|       -- CtType T1
CtCompilationUnitsModel
|
 -- CtCompilationUnit (package declaration which refers P1)
|
 -- CtCompilationUnit (refers T1)

M4) compilation units under new CtDirectory element. That would be biggest change. And there would be duplicate information - rename of package would need change of package name and change of CtDirectory name. :-(

CtModel
|
 -- CtRootPackage
|  |
|   -- CtPackage P1
|      |
|       -- CtType T1
 -- CtRootDirectory
  |
  -- CtDirectory "p1"
    |
     -- CtCompilationUnit (CtPackageDeclaration which has package reference to P1)
    |
      -- CtCompilationUnit (with type reference to T1)

Before I started to write this comment I liked the most M1, but now I am sure that I do not want to touch compilation units in future of my daily work so I prefer M2.

WDYT?

surli · 2018-10-22T18:21:54Z

Thanks for your patience Pavel :)

Ok, I indeed misunderstood some parts of the PR and so if it's not breaking I agree with you that M2 looks the better choice. But, I think you forget about the Java modules in your picture :) So for M2 be careful that the CtModel could have several modules, so maybe the CtCompilationUnit are not attached to the CtModel directly, but to their CtModule. WDYT?

pvojtechovsky · 2018-10-22T18:35:52Z

So for M2 be careful that the CtModel could have several modules, so maybe the CtCompilationUnit are not attached to the CtModel directly, but to their CtModule. WDYT?

Thanks for pointing to that. I will thing about that later. I suggest to add CtCompilationUnit as child of CtModel in different PR. This PR is already big enough.... and tests actually pass, so it is good time to review and merge it ... after other related and already made PRs are merged.

pvojtechovsky · 2018-10-23T20:08:34Z

Why CompilationUnit extends CtCompilationUnit? Why CompilationUnit is not removed?

backward compatibility
there are 146 other references to CompilationUnit in spoon sources and tests. So it would make review of this PR much harder

monperrus · 2018-10-24T18:46:36Z

src/test/java/spoon/test/compilationunit/TestCompilationUnit.java

@@ -90,11 +97,11 @@ public void testGetUnitTypeWorksWithCreatedObjects() {
 		CtPackage myPackage = launcher.getFactory().Package().getOrCreate("my.package");
 		CompilationUnit cu = launcher.getFactory().createCompilationUnit();
 		assertEquals(CompilationUnit.UNIT_TYPE.UNKNOWN, cu.getUnitType());
-
-		cu.setDeclaredPackage(myPackage);


could we stay backward compatible here and keep setDeclaredPackage?

yes, this can be derived method which will internally create a package reference and package declaration. I will do so

monperrus · 2018-10-24T18:47:19Z

src/test/java/spoon/test/compilationunit/TestCompilationUnit.java

@@ -148,7 +155,7 @@ public void testAddDeclaredTypeInCU() throws IOException {
 		assertEquals(3, cu.getDeclaredTypes().size());

 		CtType typeBla = launcher.getFactory().Class().create("spoon.test.model.Bla");
-		cu.addDeclaredType(typeBla);


could we stay backward compatible here and keep addDeclaredType?

Conceptually, we add types not references.

monperrus · 2018-10-24T18:48:51Z

In general, it looks good.

There are very few non-backward compatible changes in the tests, and it seems we may even reduce them further.

To be sure I understand well: what the final goal of this change?

pvojtechovsky · 2018-10-24T18:59:51Z

final goal of this change?

To have CtCompilationUnit, which extends CtElement and is CtVisitable. Then we can apply Processor on CtCompilationUnit. That processor will compute imports and manipulate isImplicit attribute to prepare for pretty printing.

surli · 2018-10-25T05:59:54Z

src/main/java/spoon/reflect/cu/CompilationUnit.java

@@ -128,6 +33,7 @@
 	 * 		an arbitrary index in the source code
 	 * @return the index where the line starts
 	 */
+	@Deprecated


Could you specify on the deprecated method what should be used instead with a deprecated annotation in the javadoc.

there is no better alternative for that unused (useless?) method. But may be it is safer to keep it there - without deprecated. WDYT?

Can't you get it using the position somehow?

surli · 2018-10-25T06:00:35Z

src/main/java/spoon/reflect/declaration/CtCompilationUnit.java

+ * Defines a compilation unit. In Java, a compilation unit can contain only one
+ * public type declaration and other secondary types declarations (not public).
+ */
+public interface CtCompilationUnit extends CtElement {


I would have put an @Experimental annotation: it still can change.

surli · 2018-10-25T06:02:05Z

src/main/java/spoon/reflect/factory/CompilationUnitFactory.java

@@ -75,8 +76,9 @@ public CompilationUnit getOrCreate(CtPackage ctPackage) {
 				String path = file.getCanonicalPath();
 				CompilationUnit result = this.getOrCreate(path);
 				result.setDeclaredPackage(ctPackage);
-				ctPackage.setPosition(this.factory.createPartialSourcePosition(result));
-
+				if (ctPackage.getPosition() == SourcePosition.NOPOSITION) {


Why those new checks?

This method creates CompilationUnit for an existing CtPackage. The idea was that such existing CtPackage might have correct SourcePosition... but it is probably impossible. So I will rollback those new checks

surli · 2018-10-25T06:03:06Z

src/main/java/spoon/reflect/factory/Factory.java

-	/**
-	 * @see PackageFactory#createPackageDeclaration(String)
-	 */
-	CtPackageDeclaration createPackageDeclaration(String name);


Isn't it linked to #2707 ?

This method was added in #2707, but then I found (after some changes here) that it is no more needed, so I minimized API here.

surli · 2018-10-25T06:05:17Z

src/main/java/spoon/reflect/visitor/CtBiScannerDefault.java

+		enter(compilationUnit);
+		biScan(spoon.reflect.path.CtRole.COMMENT, compilationUnit.getComments(), other.getComments());
+		biScan(spoon.reflect.path.CtRole.ANNOTATION, compilationUnit.getAnnotations(), other.getAnnotations());
+		biScan(spoon.reflect.path.CtRole.PACKAGE_DECLARATION, compilationUnit.getPackageDeclaration(), other.getPackageDeclaration());


Why the package is not also a ref? When we discussed about the model architecture, the package got a CtRootPackage as ancestor, not a CtCompilationUnit.

It is ref. Because CtCompilationUnits refers CtPackageDeclaration (new node type), which then has CtPackageReference.

OK this is then used only if the CtCompilationUnit contains java types.
What if the CtCompilationUnit is a package-info or a module-info? Shouldn't we use inherited compilation units to manage those now?

OK this is then used only if the CtCompilationUnit contains java types.

I think it is used for package-info too. But I have not checked it deeper. It should behave same like before this PR.

OK maybe we should clarify that in a future PR.

surli · 2018-10-25T06:07:25Z

src/main/java/spoon/support/compiler/jdt/JDTCommentBuilder.java

@@ -558,7 +558,7 @@ public void scan(CtElement element) {

 		if (!spoonUnit.getDeclaredTypes().isEmpty()) {
 			findCommentParentScanner.scan(spoonUnit.getDeclaredTypes());
-		} else if (spoonUnit.getDeclaredModule() != null) {
+		} else if (spoonUnit.getDeclaredModuleReference() != null) {
 			findCommentParentScanner.scan(spoonUnit.getDeclaredModule());


shouldn't it be changed to spoonUnit.getDeclaredModuleReference().getDeclaration()?

It is already in

public CtModule getDeclaredModule() { return this.moduleReference != null ? this.moduleReference.getDeclaration() : null; }

Both solutions are correct. Current solution is shorter. I can change it if you like longer in this case. No problem

So you could actually keep getDeclaratedModule() both in the test != null and after. I'd prefer a consistent solution: declaredModule or declaredModuleReference but not both.

getDeclaratedModule() is more expensive then getDeclaratedModuleReference()
So I will change the line 562 to be consistent and to use getDeclaratedModuleReference

spoon-bot · 2018-10-25T17:22:37Z

API changes: 19 (Detected by Revapi)

Old API: fr.inria.gforge.spoon:spoon-core:jar:7.2.0-20181025.162741-53 / New API: fr.inria.gforge.spoon:spoon-core:jar:7.2.0-SNAPSHOT

	Class was removed.
Old	enum CompilationUnit.UNIT_TYPE
New	none
Breaking	binary: breaking

	The type of the field changed from 'spoon.reflect.cu.CompilationUnit' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	field DefaultJavaPrettyPrinter.sourceCompilationUnit
New	field DefaultJavaPrettyPrinter.sourceCompilationUnit
Breaking	binary: breaking

	The return type changed from 'void' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	method CompilationUnit#addDeclaredType(CtType)
New	method CtCompilationUnit#addDeclaredType(CtType)
Breaking	binary: breaking
	The method used to be declared in class 'spoon.reflect.cu.CompilationUnit' but is now declared in superclass 'spoon.reflect.declaration.CtCompilationUnit'.
:---:	:---:
Old	method CompilationUnit#addDeclaredType(CtType)
New	method CtCompilationUnit#addDeclaredType(CtType)
Breaking	binary: equivalent
	Element newly annotated with 'spoon.support.DerivedProperty'.
:---:	:---:
Old	method CompilationUnit#addDeclaredType(CtType)
New	method CtCompilationUnit#addDeclaredType(CtType)
Breaking	binary: equivalent,

	Method was removed.
Old	method Factory#createPackageDeclaration(String)
New	none
Breaking	binary: breaking

	The return type changed from 'java.util.Set<spoon.reflect.declaration.CtImport>' to 'java.util.List<spoon.reflect.declaration.CtImport>'.
Old	method CompilationUnit#getImports()
New	method CtCompilationUnit#getImports()
Breaking	binary: breaking
	The method used to be declared in class 'spoon.reflect.cu.CompilationUnit' but is now declared in superclass 'spoon.reflect.declaration.CtCompilationUnit'.
:---:	:---:
Old	method CompilationUnit#getImports()
New	method CtCompilationUnit#getImports()
Breaking	binary: equivalent
	Element newly annotated with 'spoon.reflect.annotations.PropertyGetter'.
:---:	:---:
Old	method CompilationUnit#getImports()
New	method CtCompilationUnit#getImports()
Breaking	binary: equivalent,

	The return type changed from 'spoon.reflect.cu.CompilationUnit.UNIT_TYPE' to 'spoon.reflect.declaration.CtCompilationUnit.UNIT_TYPE'.
Old	method CompilationUnit#getUnitType()
New	method CtCompilationUnit#getUnitType()
Breaking	binary: breaking
	The method used to be declared in class 'spoon.reflect.cu.CompilationUnit' but is now declared in superclass 'spoon.reflect.declaration.CtCompilationUnit'.
:---:	:---:
Old	method CompilationUnit#getUnitType()
New	method CtCompilationUnit#getUnitType()
Breaking	binary: equivalent

	The return type changed from 'void' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	method CompilationUnit#setDeclaredModule(CtModule)
New	method CtCompilationUnit#setDeclaredModule(CtModule)
Breaking	binary: breaking
	The method used to be declared in class 'spoon.reflect.cu.CompilationUnit' but is now declared in superclass 'spoon.reflect.declaration.CtCompilationUnit'.
:---:	:---:
Old	method CompilationUnit#setDeclaredModule(CtModule)
New	method CtCompilationUnit#setDeclaredModule(CtModule)
Breaking	binary: equivalent
	Element newly annotated with 'spoon.support.DerivedProperty'.
:---:	:---:
Old	method CompilationUnit#setDeclaredModule(CtModule)
New	method CtCompilationUnit#setDeclaredModule(CtModule)
Breaking	binary: equivalent,

	The return type changed from 'void' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	method CompilationUnit#setDeclaredPackage(CtPackage)
New	method CtCompilationUnit#setDeclaredPackage(CtPackage)
Breaking	binary: breaking
	The method used to be declared in class 'spoon.reflect.cu.CompilationUnit' but is now declared in superclass 'spoon.reflect.declaration.CtCompilationUnit'.
:---:	:---:
Old	method CompilationUnit#setDeclaredPackage(CtPackage)
New	method CtCompilationUnit#setDeclaredPackage(CtPackage)
Breaking	binary: equivalent
	Element newly annotated with 'spoon.support.DerivedProperty'.
:---:	:---:
Old	method CompilationUnit#setDeclaredPackage(CtPackage)
New	method CtCompilationUnit#setDeclaredPackage(CtPackage)
Breaking	binary: equivalent,

	The return type changed from 'void' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	method CompilationUnit#setDeclaredTypes(List)
New	method CtCompilationUnit#setDeclaredTypes(List)
Breaking	binary: breaking
	The method used to be declared in class 'spoon.reflect.cu.CompilationUnit' but is now declared in superclass 'spoon.reflect.declaration.CtCompilationUnit'.
:---:	:---:
Old	method CompilationUnit#setDeclaredTypes(List)
New	method CtCompilationUnit#setDeclaredTypes(List)
Breaking	binary: equivalent
	Element newly annotated with 'spoon.support.DerivedProperty'.
:---:	:---:
Old	method CompilationUnit#setDeclaredTypes(List)
New	method CtCompilationUnit#setDeclaredTypes(List)
Breaking	binary: equivalent,

	The return type changed from 'void' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	method CompilationUnit#setFile(File)
New	method CtCompilationUnit#setFile(File)
Breaking	binary: breaking
	The method used to be declared in class 'spoon.reflect.cu.CompilationUnit' but is now declared in superclass 'spoon.reflect.declaration.CtCompilationUnit'.
:---:	:---:
Old	method CompilationUnit#setFile(File)
New	method CtCompilationUnit#setFile(File)
Breaking	binary: equivalent

	The return type changed from 'void' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	method CompilationUnit#setImports(Set)
New	method CtCompilationUnit#setImports(Collection)
Breaking	binary: breaking
	Element newly annotated with 'spoon.reflect.annotations.PropertySetter'.
:---:	:---:
Old	method CompilationUnit#setImports(Set)
New	method CtCompilationUnit#setImports(Collection)
Breaking	binary: equivalent,

	The return type changed from 'void' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	method CompilationUnit#setLineSeparatorPositions(.Array)
New	method CtCompilationUnit#setLineSeparatorPositions(.Array)
Breaking	binary: breaking
	The method used to be declared in class 'spoon.reflect.cu.CompilationUnit' but is now declared in superclass 'spoon.reflect.declaration.CtCompilationUnit'.
:---:	:---:
Old	method CompilationUnit#setLineSeparatorPositions(.Array)
New	method CtCompilationUnit#setLineSeparatorPositions(.Array)
Breaking	binary: equivalent

	Method was added to an interface.
Old	none
New	method CtVisitor#visitCtCompilationUnit(CtCompilationUnit)
Breaking	binary: non_breaking,

	The type of the parameter changed from 'spoon.reflect.cu.CompilationUnit' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	parameter DefaultJavaPrettyPrinter#calculate(===CompilationUnit===, List)
New	parameter DefaultJavaPrettyPrinter#calculate(===CtCompilationUnit===, List)
Breaking	binary: breaking

	The type of the parameter changed from 'spoon.reflect.cu.CompilationUnit' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	parameter ElementPrinterHelper#visitCtNamedElement(CtNamedElement, ===CompilationUnit===)
New	parameter ElementPrinterHelper#visitCtNamedElement(CtNamedElement, ===CtCompilationUnit===)
Breaking	binary: breaking

	The type of the parameter changed from 'java.util.Set<spoon.reflect.declaration.CtImport>' to 'java.lang.Iterable<spoon.reflect.declaration.CtImport>'.
Old	parameter ImportScanner#initWithImports(===Set===)
New	parameter ImportScanner#initWithImports(===Iterable===)
Breaking	binary: breaking

	The type of the parameter changed from 'spoon.reflect.cu.CompilationUnit' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	parameter PrettyPrinter#calculate(===CompilationUnit===, List)
New	parameter PrettyPrinter#calculate(===CtCompilationUnit===, List)
Breaking	binary: breaking

	The type of the parameter changed from 'spoon.reflect.cu.CompilationUnit' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	parameter PrinterHelper#mapLine(CtElement, ===CompilationUnit===)
New	parameter PrinterHelper#mapLine(CtElement, ===CtCompilationUnit===)
Breaking	binary: breaking

	The type of the parameter changed from 'spoon.reflect.cu.CompilationUnit' to 'spoon.reflect.declaration.CtCompilationUnit'.
Old	parameter SniperJavaPrettyPrinter#calculate(===CompilationUnit===, List)
New	parameter SniperJavaPrettyPrinter#calculate(===CtCompilationUnit===, List)
Breaking	binary: breaking

pvojtechovsky · 2018-10-25T18:12:45Z

All the review comments should be solved. Just unused (actually Deprecated) methods in CompilationUnit needs, which belongs to some utils class are still deprecated.

monperrus · 2018-10-25T19:24:16Z

Thanks Pavel for the heavy-duty work.

pvojtechovsky force-pushed the feaCompUnitIsElement branch from c6c2b23 to 107b560 Compare October 20, 2018 18:24

This was referenced Oct 21, 2018

fix: MetamodelProperty#containerKindOf knows Collection now #2705

Merged

fix: relax intercession test to fit #2615 - new fluent API return types #2706

Merged

feat: CtPackageDeclaration #2707

Merged

pvojtechovsky force-pushed the feaCompUnitIsElement branch from b8388d2 to 5c32bbe Compare October 21, 2018 08:47

pvojtechovsky changed the title ~~feat: CtCompilationUnit extends CtElement~~ WiP feat: CtCompilationUnit extends CtElement Oct 21, 2018

surli added the breaking_pr label Oct 21, 2018

pvojtechovsky force-pushed the feaCompUnitIsElement branch from 5c32bbe to cbbe61a Compare October 22, 2018 19:25

pvojtechovsky added 2 commits October 23, 2018 22:03

simplify package factory - delete unused API method

a3d3d59

feat: CtCompilationUnit extends CtElement

1838af2

pvojtechovsky force-pushed the feaCompUnitIsElement branch from cbbe61a to 1838af2 Compare October 23, 2018 20:04

pvojtechovsky added the review label Oct 24, 2018

pvojtechovsky changed the title ~~WiP feat: CtCompilationUnit extends CtElement~~ feat: CtCompilationUnit extends CtElement Oct 24, 2018

monperrus reviewed Oct 24, 2018

View reviewed changes

make it more backward compatible

983b221

surli reviewed Oct 25, 2018

View reviewed changes

Pavel Vojtěchovský and others added 3 commits October 25, 2018 08:26

fixed review problems

6fb710d

fixes

2f4803b

fix

9ac4a8f

monperrus merged commit 96d3150 into INRIA:master Oct 25, 2018

pvojtechovsky deleted the feaCompUnitIsElement branch October 25, 2018 19:41

slarse mentioned this pull request May 24, 2021

Add CtElement.getCompilationUnit method to the public API #3942

Open

slarse mentioned this pull request Dec 25, 2022

[Question] Is there a specific reason the CompilationUnit type exists? #5039

Closed

feat: CtCompilationUnit extends CtElement #2702

feat: CtCompilationUnit extends CtElement #2702

Conversation

pvojtechovsky commented Oct 20, 2018 • edited Loading

surli commented Oct 21, 2018

pvojtechovsky commented Oct 21, 2018

surli commented Oct 22, 2018

pvojtechovsky commented Oct 22, 2018

surli commented Oct 22, 2018

pvojtechovsky commented Oct 22, 2018

surli commented Oct 22, 2018

pvojtechovsky commented Oct 22, 2018 • edited Loading

surli commented Oct 22, 2018

pvojtechovsky commented Oct 22, 2018

pvojtechovsky commented Oct 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monperrus commented Oct 24, 2018

pvojtechovsky commented Oct 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spoon-bot commented Oct 25, 2018

pvojtechovsky commented Oct 25, 2018

monperrus commented Oct 25, 2018

pvojtechovsky commented Oct 20, 2018 •

edited

Loading

pvojtechovsky commented Oct 22, 2018 •

edited

Loading

pvojtechovsky commented Oct 23, 2018 •

edited

Loading