Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 325464

Summary: [editor] IProblemRequestor does not get installed on JS file working copy when new JS File wizard is used
Product: [WebTools] JSDT Reporter: Ian Tewksbury <itewksbu>
Component: GeneralAssignee: Ian Tewksbury <itewksbu>
Status: RESOLVED FIXED QA Contact: Nitin Dahyabhai <thatnitind>
Severity: normal    
Priority: P3 CC: cmjaun
Version: 3.2.2Flags: thatnitind: review+
Target Milestone: 3.2.3   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on:    
Bug Blocks: 326605    
Attachments:
Description Flags
Fix Patch
none
Fix Patch - Update 1 none

Description Ian Tewksbury CLA 2010-09-16 09:37:08 EDT
STEPS:
1. create a SWP
2. create a JS file using the new JS file wizard
3. add a single double quote to the document (syntax error)

RESULTS:
1. without saving the document the syntax validation error should be reported by its not

CAUSE:
this is because in NewJSFileWizardPage@#addFileComment the line: "cu.becomeWorkingCopy(new NullProgressMonitor());" (added by Bug 268125)

This causes the working copy to be created without a problem requester. Then when CompilationUnitDocumentProvider#createFileInfo is called there is a bit of code that reads:
/*
* Use the deprecated method to ensure that our problem requestor is
* used; it is the only way to have as-you-type IProblems from
* reconciling appear in the annotation model.
*/
if (JavaModelUtil.isPrimary(original))
  original.becomeWorkingCopy(requestor, getProgressMonitor());

This attempts to set up the working copy again using the correct requestor, problem is in CompilationUnit#becomeWorkingCopy(IProblemRequestor, IProgressMonitor) if the PerWorkignCopyInfo already exists then nothing is done.
Comment 1 Ian Tewksbury CLA 2010-09-16 09:41:35 EDT
Created attachment 179033 [details]
Fix Patch

This patch is a candidate solution to the problem.  All it does is update CompilationUnit#becomeWorkingCopy so that if JavaModelManager.PerWorkingCopyInfo already exists it updates perWorkingCopyInfo.problemRequestor with the given problemReqestor.

I say candidate solution because I am not sure the legitimacy of this approach. It does fix the issue and it does pass all existing JUnits, but it still seems a bit wonky to me.  Any other suggestions would be welcome.
Comment 2 Ian Tewksbury CLA 2010-09-16 09:46:44 EDT
FYI: the code that adds the correct requester was added back in Bug 311990
Comment 3 Nitin Dahyabhai CLA 2010-09-16 11:25:44 EDT
Any reason we don't just either have the wizard discard its working copy?  What would this patch do if a problem requestor was already in place?
Comment 4 Ian Tewksbury CLA 2010-09-16 12:47:20 EDT
(In reply to comment #3)
> Any reason we don't just either have the wizard discard its working copy?

I like the sound of that idea, how does one do that?

> What would this patch do if a problem requestor was already in place?

hence my not loving this patch, it would just wipe it out.
Comment 5 Ian Tewksbury CLA 2010-09-17 09:16:05 EDT
Created attachment 179110 [details]
Fix Patch - Update 1

(In reply to comment #3)
> Any reason we don't just either have the wizard discard its working copy?

That worked great.  And I like it much better then my original patch.
Here is an updated patch that discards the working copy after adding the file comment.
Comment 6 Chris Jaun CLA 2010-09-29 17:53:08 EDT
Patch checked into 3.2.3 and HEAD.