Bug 307295 - Task tags and task priorities
Summary: Task tags and task priorities
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.6   Edit
Hardware: All Linux
: P3 normal (vote)
Target Milestone: 3.6 M7   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 307742 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-03-27 17:26 EDT by Marco Ippolito CLA
Modified: 2010-04-27 03:45 EDT (History)
4 users (show)

See Also:


Attachments
Proposed fix (10.18 KB, patch)
2010-04-07 21:24 EDT, Olivier Thomann CLA
no flags Details | Diff
Proposed fix (10.18 KB, patch)
2010-04-08 13:54 EDT, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marco Ippolito CLA 2010-03-27 17:26:43 EDT
Build Identifier: Eclipse 3.5.2 Build ID: M20100211-1343

In JDT, inadvertently defining a number of task tags different from the number of defined task priorities has cascading effects on the whole JDT, with code completion being the first functionality to break.

The exception stack traces may seem "mystical" at first, not giving away immediately what's failing.

Please see "Steps to Reproduce" for more details.



Reproducible: Always

Steps to Reproduce:
Preferences changes to replicate the problem:

/instance/org.eclipse.jdt.core/org.eclipse.jdt.core.compiler.taskPriorities=NORMAL,HIGH,NORMAL,LOW,HIGH,HIGH
/instance/org.eclipse.jdt.core/org.eclipse.jdt.core.compiler.taskTags=TODO,MUSTDO,SHOULDDO,COULDDO,FIXME,DELME,TODOC    

Note: there are less taskPriorities than taskTags.
Comment 1 Dani Megert CLA 2010-03-29 03:27:31 EDT
>The exception stack traces may seem "mystical" at first, not giving away
>immediately what's failing.
Please attach it here.

This looks like a bug in JDT Core. The Javadoc for those constants is not clearly specified either.
Comment 2 Marco Ippolito CLA 2010-03-29 07:34:32 EDT
(In reply to comment #1)
> >The exception stack traces may seem "mystical" at first, not giving away
> >immediately what's failing.
> Please attach it here.
> 
> This looks like a bug in JDT Core. The Javadoc for those constants is not
> clearly specified either.

Here's a sample exception stack trace that only happens with unbalances task tags / priorities and disappears when balance is restored:

Exception in thread "AutoAssist Delay" org.eclipse.swt.SWTException: Failed to execute runnable (java.lang.ArrayIndexOutOfBoundsException: 6)
	at org.eclipse.swt.SWT.error(SWT.java:3884)
	at org.eclipse.swt.SWT.error(SWT.java:3799)
	at org.eclipse.swt.widgets.Synchronizer.syncExec(Synchronizer.java:195)
	at org.eclipse.ui.internal.UISynchronizer.syncExec(UISynchronizer.java:150)
	at org.eclipse.swt.widgets.Display.syncExec(Display.java:4113)
	at org.eclipse.jface.text.contentassist.ContentAssistant$AutoAssistListener.showAssist(ContentAssistant.java:365)
	at org.eclipse.jface.text.contentassist.ContentAssistant$AutoAssistListener.run(ContentAssistant.java:277)
	at java.lang.Thread.run(Thread.java:636)
Caused by: java.lang.ArrayIndexOutOfBoundsException: 6
	at org.eclipse.jdt.internal.compiler.parser.Scanner.<init>(Scanner.java:223)
	at org.eclipse.jdt.internal.compiler.parser.Scanner.<init>(Scanner.java:251)
	at org.eclipse.jdt.internal.core.util.CommentRecorderParser.initializeScanner(CommentRecorderParser.java:230)
	at org.eclipse.jdt.internal.compiler.parser.Parser.<init>(Parser.java:877)
	at org.eclipse.jdt.internal.core.util.CommentRecorderParser.<init>(CommentRecorderParser.java:39)
	at org.eclipse.jdt.internal.compiler.SourceElementParser.<init>(SourceElementParser.java:79)
	at org.eclipse.jdt.internal.compiler.SourceElementParser.<init>(SourceElementParser.java:63)
	at org.eclipse.jdt.internal.core.SourceMapper.mapSource(SourceMapper.java:1234)
	at org.eclipse.jdt.internal.core.SourceMapper.mapSource(SourceMapper.java:1176)
	at org.eclipse.jdt.internal.core.BinaryMethod.getParameterNames(BinaryMethod.java:167)
	at org.eclipse.jdt.internal.codeassist.InternalCompletionProposal.findMethodParameterNames(InternalCompletionProposal.java:317)
	at org.eclipse.jdt.internal.codeassist.InternalCompletionProposal.findParameterNames(InternalCompletionProposal.java:1416)
	at org.eclipse.jdt.ui.text.java.CompletionProposalLabelProvider.appendUnboundedParameterList(CompletionProposalLabelProvider.java:109)
	at org.eclipse.jdt.ui.text.java.CompletionProposalLabelProvider.createMethodProposalLabel(CompletionProposalLabelProvider.java:268)
	at org.eclipse.jdt.ui.text.java.CompletionProposalLabelProvider.createStyledLabel(CompletionProposalLabelProvider.java:558)
	at org.eclipse.jdt.internal.ui.text.java.LazyJavaCompletionProposal.computeDisplayString(LazyJavaCompletionProposal.java:247)
	at org.eclipse.jdt.internal.ui.text.java.LazyJavaCompletionProposal.getStyledDisplayString(LazyJavaCompletionProposal.java:226)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.handleSetData(CompletionProposalPopup.java:827)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.access$24(CompletionProposalPopup.java:817)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup$3.handleEvent(CompletionProposalPopup.java:583)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1176)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1200)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1185)
	at org.eclipse.swt.widgets.Table.checkData(Table.java:286)
	at org.eclipse.swt.widgets.Table.cellDataProc(Table.java:223)
	at org.eclipse.swt.widgets.Display.cellDataProc(Display.java:707)
	at org.eclipse.swt.internal.gtk.OS._gtk_main_do_event(Native Method)
	at org.eclipse.swt.internal.gtk.OS.gtk_main_do_event(OS.java:7603)
	at org.eclipse.swt.widgets.Display.eventProc(Display.java:1185)
	at org.eclipse.swt.internal.gtk.OS._g_main_context_iteration(Native Method)
	at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(OS.java:1860)
	at org.eclipse.swt.widgets.Shell.setVisible(Shell.java:1781)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.displayProposals(CompletionProposalPopup.java:1192)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.access$23(CompletionProposalPopup.java:1157)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup$2.run(CompletionProposalPopup.java:500)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.showProposals(CompletionProposalPopup.java:482)
	at org.eclipse.jface.text.contentassist.ContentAssistant$2.run(ContentAssistant.java:376)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:134)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3468)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3115)
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2405)
	at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2369)
	at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2221)
	at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:500)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:493)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:113)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:194)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:368)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:179)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:616)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:559)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:514)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1311)

On occasion problems propagate to the IDE itself, disabling buttons, invalidating actions, etc.

Should I try to create a patch for this myself, just containing the brittleness of preferences parsing?
Comment 3 Olivier Thomann CLA 2010-03-29 10:32:04 EDT
Did you update manually the preference file ?
Comment 4 Dani Megert CLA 2010-03-29 10:35:11 EDT
>Did you update manually the preference file ?
Might be - or use the official API to set JDT Core options.
Comment 5 Olivier Thomann CLA 2010-03-30 09:38:09 EDT
My concern is about the way we can provide feedback to the user to explain the problem when both sizes are different.
We can clearly improve the javadoc of both constants.
This being done, we should still prevent the exception to occur.
The best way would be to throw an IllegalArgumentException, but the API doesn't expect an exception to be thrown.
Comment 6 Dani Megert CLA 2010-03-30 09:49:48 EDT
I would improve the Javadoc and make sure that wrong values don't cause problems i.e. make sure they have the same amount of entries. In case of different entries, shorten the longer one and write to the .log that those options have illegal values.
Comment 7 Marco Ippolito CLA 2010-03-30 10:12:16 EDT
(In reply to comment #6)
> I would improve the Javadoc and make sure that wrong values don't cause
> problems i.e. make sure they have the same amount of entries. In case of
> different entries, shorten the longer one and write to the .log that those
> options have illegal values.

Re-balancing tags and priorities with a default setting while logging an error does make sense. Containment of the propagation of such an error to the operation of the whole IDE seems even higher priority. Revision of other cases in which this may occur (spurious preferences crashing the GUI)  could become a TODO task tag, with an unbalanced priority. ;-)
Comment 8 Dani Megert CLA 2010-03-30 10:21:11 EDT
How did you set those options? It's not possible via UI.

We won't do more than outlined in comment 6 as this is overkill for such a rare case.
Comment 9 Marco Ippolito CLA 2010-03-30 10:26:07 EDT
(In reply to comment #8)
> How did you set those options? It's not possible via UI.
> 
> We won't do more than outlined in comment 6 as this is overkill for such a rare
> case.

It occurred through preferences file export / import. What's outlined in comment 6 seems like the perfect approach and more than sufficient...
Comment 10 Dani Megert CLA 2010-03-30 10:33:37 EDT
>It occurred through preferences file export / import.
But you modified the prefs right? Otherwise it would be a bug in export and/or import.
Comment 11 Marco Ippolito CLA 2010-03-30 10:47:29 EDT
(In reply to comment #10)
> >It occurred through preferences file export / import.
> But you modified the prefs right? Otherwise it would be a bug in export and/or
> import.

Yes, I did. It's not a bug in the export/import.
Comment 12 Olivier Thomann CLA 2010-03-31 16:40:30 EDT
*** Bug 307742 has been marked as a duplicate of this bug. ***
Comment 13 Olivier Thomann CLA 2010-04-07 21:24:57 EDT
Created attachment 164157 [details]
Proposed fix
Comment 14 Olivier Thomann CLA 2010-04-08 13:54:54 EDT
Created attachment 164260 [details]
Proposed fix
Comment 15 Olivier Thomann CLA 2010-04-08 13:58:58 EDT
Released for 3.6M7.
Comment 16 Dani Megert CLA 2010-04-09 04:03:49 EDT
The fix is not good: it causes and NPE and results in many test failures in JDT Text and UI tests.

NPE happens on line 3611 of org.eclipse.jdt.internal.core.util.Util.fixTaskTags(Map).
Comment 17 Dani Megert CLA 2010-04-09 05:58:23 EDT
The 'return' needs to be shifted outside the block:

if (taskPriorities == null) {
	if (taskTags != null) {
		Util.logRepeatedMessage(...);
		defaultOptionsMap.remove(JavaCore.COMPILER_TASK_TAGS);
		return;
	}
} else if (taskTags == null) {

==>

if (taskPriorities == null) {
	if (taskTags != null) {
		Util.logRepeatedMessage(...);
		defaultOptionsMap.remove(JavaCore.COMPILER_TASK_TAGS);
	}
	return;
} else if (taskTags == null) {
Comment 18 Olivier Thomann CLA 2010-04-09 09:38:37 EDT
Fixed in HEAD.
Added regression test in:
org.eclipse.jdt.core.tests.model.JavaCoreOptionsTests
Comment 19 Jay Arthanareeswaran CLA 2010-04-27 03:45:37 EDT
Verified for 3.6M7 using build I20100424-2000.