Community
Participate
Working Groups
20051109 I had a first look at the new API. This are my first remarks and questions 1. Termwise, it feels to me we could simplify things by using either 'compilation' or 'reconcile' (-> use ReconcileParticipant / ReconcileContext or CompilationParticipation/CompilationContext). I would suggest the first one. Compilation sounds more like byte code assembly and that wouldn't apply to the pure editor reconcile. 2. As the TODO states in ReconcileContext.getAST, it looks strange that a client can ask for an AST of a specific level. Wouldn't that result in bad performance as an AST might have to be build several times when an old client is around? I would spec here that the client has to check for the level and pass when the level is unknown to it. 3. It is not clear to me what happens when you do clearAST. Will this restart the whole reconcile, restarting all participants? A clear would make all previous analysis and detected problems obsolete. Wouldn't this better be expressed in CompilationParticipant.reconcile: boolean, where e.g. returning false would mean that the reconcile has to restart. What happens to getDelta() after a clearAST(). Would it be a combined delta since the start of the reconcile? 4. Is the 'putProblems' really intended to replace all existing markers of type. Does that mean I could replace all jdt.core problems? 5. Note that getAST() could probably also be expressed with getDelta().getCompilationUnitAST() and getWorkingCopy() with (CompilationUnit) getDelta().getElement()
Re:1. The idea is that if you want to play in compilation, you either want to participate in building and/or reconciling; since compilation occurs in these 2 stages mostly. Do we need to make them separate or consider them as two facets of the same problem ? Reconcile is doing full compilation. Re:2. I had the same comment to Jerome. But looking at the API, the AST level is something clients can express when reconciling. I would have been more aggressive on saying that participants are only performing on AST3... but this is maybe to harsh, especially when you want to mix with legacy plugins (e.g. what if someone calls reconcile(AST2)). Re:3. #clearAST() is simply telling to the context to flush the cached AST, since it got invalidated. Maybe #flush/discardAST would be a better name. The next getAST query will thus force to recreate a fresh new one. If no one asks for a subsequent AST, then it has no performance penalty. And it wouldn't re-iterate the reconcile action. Deltas are intended to be batched throughout the reconcile operation. Re:4. I believe so. Some participants may exactly want to do this (e.g. filter certain categories of problems: hide deprecation warnings related to a specific component usage). The theory is that orthogonal clients are using different marker types, so they wouldn't interfere. Re:5. Interesting thoughts. I let Jerome answer it. Jerome: I think these questions would be best answered directly into the API spec, they are indeed valid questions anyone could come up with when trying to use this new API.
3. Assume we have several participants, if participant 2 'invalidates' an AST, wouldn't that mean it either changed something in the source or in the environment (e.g. new type generated). And that could influence the results that participant 1 just provided. I think participant 1 need to be ran again as its analysis might not be valid anymore. (I could think of e.g. the jdt.core error checker being the participant 1)
The reconcile AST is read-only. The Java participation is only running at the end. Now side effects could still occur on the buffer itself, which would cause to reset the AST implicitly (I guess). Also a DOM AST could be invalidated (bindings) by generation of derived types during reconcile participation (e.g. APT). Now if the buffer gets modified, running the reconcile again is likely something dangerous (could never complete). I would rather favor preventing clients to alter the reconciled buffer during the process to keep things simple and predictable.
(In reply to comment #0) > 5. Note that getAST() could probably also be expressed with > getDelta().getCompilationUnitAST() The AST is not yet positionnned on the delta at this stage. I will update the spec to clarify this point. > and getWorkingCopy() with (CompilationUnit) getDelta().getElement() Since the delta's root is not spec'ed to be the working copy, the correct code would be: IJavaElement element = getDelta.getElement(); if (element.getElementType() == IJavaElement.COMPILATION_UNIT) return (ICompilationUnit) element; else // walk the delta to find the working copy .... So you can see getWorkingCopy() as a good helper.
Thanks for the feedback Martin. I modified the following to clarify things: 1. Clarified what is the compilation process in CompilationParticipant 2. Added getASTLevel() and isResolvingBindings() on ReconcileContext so that clients can know what is the current level and binding resolution. Added clarification in getAST(int, boolean) that a new temporary AST is created if level/resoultion is different. 3. Clarified the intent of resetAST() 4. Clarified the intent of putProblems(...) 5. Spec'ed getDelta() to say that the AST should be obtained through getAST(...)
Thanks Jerome. The new methods make sense. I'm still puzzled with clearAST: 'resetting the AST will not restart the reconcile process'. So what about the results of the previous reconcile participants that became obsolete with the changes of buffer content or environment? Showing them can result in strange highlighting.
As Philippe said, if we restarted the reconcile process we could end up in infinite loops. As for highligthing, problems are reported at the end of the reconcile to the requestor, so you would not see strange highlighting.
They are reported at the end, but who would correct the offsets of the problems to the AST before the change? Wouldn't it be better to forbid participants to change the buffer?
Changed spec to forbid participants from changing the buffer.
Actually, a participant modifying the model would result in the same problem. So not allowing a participant to modify the buffer is not the solution. Instead I added a configure(List) method on CompilationParticipant that allows a participant to define the order in which it wants to be called. If the participant is modifying the model or the buffer (write participant), it should put itself first in the list. If it just creates problems from a consistent view of the world (read participant), it should put itself last.
>Changed spec to forbid participants from changing the buffer. Jerome, do you now prevent this by using a read-only buffer? In addition we should do the following: You catch 'UnsupportedOperationException' when calling a participant and the reconciler throws this exception in case when a participant modifies the original working copy. If you get the exception you write a log entry and disable that participant. This is better than deactivating the whole reconcile operation by the reconciler.
After more discussion, we came to the conclusion that the configure(List) solution was not good enough. Instead changed the extension point to specify whether the participant modifies the environment, whether it creates problems, and optionally the list of participants it requires to run before itself. The spec now says that the participant should not modify the buffer of the working copy being reconciled, and as Dani suggested, we now catch UnsupportedOperationException.
The reconciler now detects this and throws UOE.
Verified for 3.2 M4 using build I20051213-0010
