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

Bug 328601

Summary: [nls tooling] NLS Hover writes IAE to log
Product: [Eclipse Project] JDT Reporter: Markus Keller <markus.kell.r>
Component: TextAssignee: Deepak Azad <deepakazad>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P2 CC: daniel_megert
Version: 3.7Flags: markus.kell.r: review+
Target Milestone: 3.7 M6   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
Javadoc hover with warning
none
screenshot with warning
none
proposed patch
none
screenshot with warning II
none
proposed patch II
none
additional fix none

Description Markus Keller CLA 2010-10-25 09:36:38 EDT 
HEAD, probably an older problem

- Create a .properties file with invalid content like this:
Snippet.xxx=\u

- Hover over any key that refers to that .properties file
=> IAE below is logged
=> Expected: Nothing in log, but hover should tell which .properties file is malformed.

java.lang.IllegalArgumentException: Malformed \uxxxx encoding.
	at java.util.Properties.loadConvert(Properties.java:552)
	at java.util.Properties.load0(Properties.java:375)
	at java.util.Properties.load(Properties.java:325)
	at org.eclipse.jdt.internal.corext.refactoring.nls.NLSHintHelper.getProperties(NLSHintHelper.java:461)
	at org.eclipse.jdt.internal.ui.text.java.hover.NLSStringHover.internalGetHoverInfo(NLSStringHover.java:171)
	at org.eclipse.jdt.internal.ui.text.java.hover.NLSStringHover.getHoverInfo2(NLSStringHover.java:112)
	at org.eclipse.jdt.internal.ui.text.java.hover.BestMatchHover.getHoverInfo2(BestMatchHover.java:141)
	at org.eclipse.jdt.internal.ui.text.java.hover.JavaEditorTextHoverProxy.getHoverInfo2(JavaEditorTextHoverProxy.java:82)
	at org.eclipse.jface.text.TextViewerHoverManager$4.run(TextViewerHoverManager.java:166)
Comment 1 Deepak Azad CLA 2010-10-25 09:55:15 EDT 
(In reply to comment #0)
> HEAD, probably an older problem
Yup, it is an older problem. And I think you get the same IAE in other parts of the tooling as well. I will take a look.
Comment 2 Dani Megert CLA 2010-12-16 07:18:43 EST 
Created attachment 185307 [details]
Javadoc hover with warning
Comment 3 Dani Megert CLA 2010-12-16 07:19:03 EST 
> => Expected: Nothing in log, but hover should tell which .properties file is
> malformed.
Agreed, but the text should be in italic, like in the Javadoc hover when there's no font (see attachment 185307 [details]).
Comment 4 Markus Keller CLA 2011-02-09 05:46:32 EST 
(In reply to comment #3)
> Agreed, but the text should be in italic, like in the Javadoc hover when
> there's no font (see attachment 185307 [details]).

That's not trivial, since HTML2TextReader currently doesn't support italics.
A formatting like that for missing keys would also be okay for me.
Comment 5 Deepak Azad CLA 2011-02-13 22:23:40 EST 
Created attachment 188866 [details]
screenshot with warning

(In reply to comment #4)
> (In reply to comment #3)
> > Agreed, but the text should be in italic, like in the Javadoc hover when
> > there's no font (see attachment 185307 [details] [details]).
> 
> That's not trivial, since HTML2TextReader currently doesn't support italics.
> A formatting like that for missing keys would also be okay for me.

The attached screenshot is ok ?
Comment 6 Deepak Azad CLA 2011-02-13 22:24:29 EST 
Created attachment 188867 [details]
proposed patch
Comment 7 Markus Keller CLA 2011-02-14 06:46:28 EST 
> Created attachment 188866 [details]
> screenshot with warning

Hmm, the hover is a bit confusing, since it just talks about an invalid Unicode sequence, but does not tell where exactly the problem is. I guess you don't get the actual line number where the problem occurs, but I think we should tell explicitly that the problem is in the properties file and not in the hovered string literal. E.g. like this (also avoids the double colons in one sentence):

<b>Warning:</b> The properties file could not be read!
Invalid Unicode sequence: illegal character
Comment 8 Deepak Azad CLA 2011-02-14 12:55:47 EST 
Created attachment 188927 [details]
screenshot with warning II
Comment 9 Deepak Azad CLA 2011-02-14 12:56:32 EST 
Created attachment 188928 [details]
proposed patch II

Markus, good to go ?
Comment 10 Markus Keller CLA 2011-02-14 14:46:40 EST 
> Created attachment 188928 [details] [diff]
> proposed patch II
> 
> Markus, good to go ?

Yep, if you fix the formatting of the if-else in NLSStringHover#toHtml(..).
Comment 11 Deepak Azad CLA 2011-02-14 22:09:11 EST 
Fixed in HEAD.
Comment 12 Dani Megert CLA 2011-02-15 06:25:48 EST 
This almost works except when I click on the icon in the enriched NLS hover that now shows in the Java editor: it has "" as key and hence doesn't bring me to the right key/value pair. However, clicking on the constant works i.e. brings me to the right key in the properties file.
Comment 13 Deepak Azad CLA 2011-02-16 08:35:58 EST 
Created attachment 189092 [details]
additional fix

(In reply to comment #12)
> This almost works except when I click on the icon in the enriched NLS hover
> that now shows in the Java editor: it has "" as key and hence doesn't bring me
> to the right key/value pair. However, clicking on the constant works i.e.
> brings me to the right key in the properties file.
Thanks Dani.
Comment 14 Deepak Azad CLA 2011-02-16 08:36:33 EST 
Fixed in HEAD.
Comment 15 Dani Megert CLA 2011-02-16 11:49:49 EST 
Verified in HEAD.