Community
Participate
Working Groups
If I externalize strings using non-Eclipse-style, then change the literal key strings in the code to constants, the Find Broken Externalized Strings function reports such keys as undefined in the code, and unused in the properties file. For example, with this class, public class Main { private static final String MAIN_INDIRECT = "Main.indirect"; public static void main(String[] args) { System.out.println(Messages.getString("Main.direct")); //$NON-NLS-1$ System.out.println(Messages.getString(MAIN_INDIRECT)); //$NON-NLS-1$ } } the "Main.indirect" key is reported as undefined and unused. We have a lot of externalized strings, can't use Eclipse-style externalization, and like to have the keys defined as constants for improved readability and maintainability. It's unfortunate to lose the ability to check for broken string externalization.
+1 for this enhancement. Having to pick between improved readability and a tool that checks for NLS breakages is unfortunate.
"Find Broken Externalized Strings" is provided by JDT/UI. Moving to JDT/UI
The Eclipse externalization method does exactly that i.e. uses constants AND it is much better in terms of memory usage. For details see: http://dev.eclipse.org/viewcvs/index.cgi/platform-core-home/documents/3.1/message_bundles.html?view=co Why don't you use that one?
We have a server component that must serve multiple clients that are potentially running in different locales. We want the server to generate messages for a particular client using the client's locale, so it can't just preload all the messages using the server's locale.
OK, I see. Are those constants in the same file or would we have to check other files?
Requiring the constants to be in the same file would be fine. (We typically create them using the Extract Constant refactoring.)
Will see what I can do. 3.5 time permitting.
Sorry not time left for this.
Created attachment 150912 [details] patch I have added checks for string constants defined in the same class. (Extract constant refactoring creates the constants in the same class.)
My first try of the patch causes exceptions: !ENTRY org.eclipse.core.jobs 4 2 2009-11-02 15:36:45.683 !MESSAGE An internal error occurred during: "Search for Broken NLS Keys". !STACK 0 org.eclipse.core.runtime.AssertionFailedException: assertion failed: at org.eclipse.core.runtime.Assert.isTrue(Assert.java:110) at org.eclipse.core.runtime.Assert.isTrue(Assert.java:96) at org.eclipse.jface.text.Position.setLength(Position.java:185) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchResultRequestor.findKey(NLSSearchResultRequestor.java:248) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchResultRequestor.acceptSearchMatch(NLSSearchResultRequestor.java:122) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.report(MatchLocator.java:1760) at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.matchReportReference(TypeReferenceLocator.java:513) at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.matchReportReference(TypeReferenceLocator.java:342) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2163) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2638) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2365) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.process(MatchLocator.java:1627) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1037) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1078) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1210) at org.eclipse.jdt.internal.core.search.JavaSearchParticipant.locateMatches(JavaSearchParticipant.java:94) at org.eclipse.jdt.internal.core.search.BasicSearchEngine.findMatches(BasicSearchEngine.java:231) at org.eclipse.jdt.internal.core.search.BasicSearchEngine.search(BasicSearchEngine.java:515) at org.eclipse.jdt.core.search.SearchEngine.search(SearchEngine.java:582) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchQuery.run(NLSSearchQuery.java:90) at org.eclipse.search2.internal.ui.InternalSearchUI$InternalSearchJob.run(InternalSearchUI.java:91) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55) !ENTRY org.eclipse.core.jobs 4 2 2009-11-02 15:36:45.683 !MESSAGE An internal error occurred during: "Search for Broken NLS Keys". !STACK 0 org.eclipse.core.runtime.AssertionFailedException: assertion failed: at org.eclipse.core.runtime.Assert.isTrue(Assert.java:110) at org.eclipse.core.runtime.Assert.isTrue(Assert.java:96) at org.eclipse.jface.text.Position.setLength(Position.java:185) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchResultRequestor.findKey(NLSSearchResultRequestor.java:248) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchResultRequestor.acceptSearchMatch(NLSSearchResultRequestor.java:122) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.report(MatchLocator.java:1760) at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.matchReportReference(TypeReferenceLocator.java:513) at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.matchReportReference(TypeReferenceLocator.java:342) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2163) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2638) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2365) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.process(MatchLocator.java:1627) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1037) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1078) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1210) at org.eclipse.jdt.internal.core.search.JavaSearchParticipant.locateMatches(JavaSearchParticipant.java:94) at org.eclipse.jdt.internal.core.search.BasicSearchEngine.findMatches(BasicSearchEngine.java:231) at org.eclipse.jdt.internal.core.search.BasicSearchEngine.search(BasicSearchEngine.java:515) at org.eclipse.jdt.core.search.SearchEngine.search(SearchEngine.java:582) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchQuery.run(NLSSearchQuery.java:90) at org.eclipse.search2.internal.ui.InternalSearchUI$InternalSearchJob.run(InternalSearchUI.java:91) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)
And with the example from comment 0 I get NPEs: !ENTRY org.eclipse.core.jobs 4 2 2009-11-02 15:41:02.240 !MESSAGE An internal error occurred during: "Search for Broken NLS Keys". !STACK 0 java.lang.NullPointerException at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchResultRequestor.findKey(NLSSearchResultRequestor.java:258) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchResultRequestor.acceptSearchMatch(NLSSearchResultRequestor.java:122) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.report(MatchLocator.java:1760) at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.matchReportReference(TypeReferenceLocator.java:513) at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.matchReportReference(TypeReferenceLocator.java:342) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2163) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2638) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2365) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.process(MatchLocator.java:1627) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1037) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1078) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1195) at org.eclipse.jdt.internal.core.search.JavaSearchParticipant.locateMatches(JavaSearchParticipant.java:94) at org.eclipse.jdt.internal.core.search.BasicSearchEngine.findMatches(BasicSearchEngine.java:231) at org.eclipse.jdt.internal.core.search.BasicSearchEngine.search(BasicSearchEngine.java:515) at org.eclipse.jdt.core.search.SearchEngine.search(SearchEngine.java:582) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchQuery.run(NLSSearchQuery.java:90) at org.eclipse.search2.internal.ui.InternalSearchUI$InternalSearchJob.run(InternalSearchUI.java:91) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55) !ENTRY org.eclipse.core.jobs 4 2 2009-11-02 15:41:02.240 !MESSAGE An internal error occurred during: "Search for Broken NLS Keys". !STACK 0 java.lang.NullPointerException at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchResultRequestor.findKey(NLSSearchResultRequestor.java:258) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchResultRequestor.acceptSearchMatch(NLSSearchResultRequestor.java:122) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.report(MatchLocator.java:1760) at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.matchReportReference(TypeReferenceLocator.java:513) at org.eclipse.jdt.internal.core.search.matching.TypeReferenceLocator.matchReportReference(TypeReferenceLocator.java:342) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2163) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2638) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.reportMatching(MatchLocator.java:2365) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.process(MatchLocator.java:1627) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1037) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1078) at org.eclipse.jdt.internal.core.search.matching.MatchLocator.locateMatches(MatchLocator.java:1195) at org.eclipse.jdt.internal.core.search.JavaSearchParticipant.locateMatches(JavaSearchParticipant.java:94) at org.eclipse.jdt.internal.core.search.BasicSearchEngine.findMatches(BasicSearchEngine.java:231) at org.eclipse.jdt.internal.core.search.BasicSearchEngine.search(BasicSearchEngine.java:515) at org.eclipse.jdt.core.search.SearchEngine.search(SearchEngine.java:582) at org.eclipse.jdt.internal.ui.refactoring.nls.search.NLSSearchQuery.run(NLSSearchQuery.java:90) at org.eclipse.search2.internal.ui.InternalSearchUI$InternalSearchJob.run(InternalSearchUI.java:91) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:55)
Please also make sure to update the copyright date.
Created attachment 151156 [details] reworked patch Assertion Failed Exception (first set of exceptions) occurred when the length of identifier name was 1. eg private static final String X ="X"; I have fixed this problem The null pointer exception (second set) was unreproducible.
Deepak, please also add some test cases for this new scenario (in org.eclipse.jdt.ui.tests - org.eclipse.jdt.ui.tests.search).
Created attachment 151172 [details] fix + testcase I was able to reproduce null pointer exception from comment 11. The attached patch includes the fix for this. The patch also includes test cases for the fix.
The patch is not good as it can lead to ClassCastExceptions in case there's a 1-arg method that takes something else than a String constant, e.g. something like: public static final int MAIN_INDIRECT2 = 1; public static void main(String[] args) { System.out.println(Messages.getString(MAIN_INDIRECT2)); } public class Messages { public static String getString(int i) { return ""; } } Other minor issues with the patch: - I hate code duplication ;-) I suggest we extend the first if (!...) with the other case, then assign the common values and return found argument string for the existing case. Note that the same string can serve as the identifier in the ongoing code. - the additional Javadoc comment is not correct: the field does not need to be private - testBug247012_2 also passes without the fix - is this intentional?
Created attachment 151642 [details] fix + testcases (In reply to comment #16) > The patch is not good as it can lead to ClassCastExceptions in case there's a > 1-arg method that takes something else than a String constant, Added a check before casting to String > Other minor issues with the patch: > - I hate code duplication ;-) Improved the patch :) > - testBug247012_2 also passes without the fix - is this intentional? Yes, this is intentional. testBug247012_2 and testBug247012_3 are to test for the case when the constant is not declared as something like 'static final String MAIN_INDIRECT = "Main.indirect";'. testBug247012_2 catches the null pointer exception from comment 11 and testBug247012_3 catches the classCastException mentioned above.
The patch fixes the CCE but introduces a new bug: the returned key string is wrong in the normal case i.e. it includes the '"' at the end. In addition I see no value to loop over all fields once we've found a match i.e. simply return on first match. ==> - remove local var 'key' - return result from if and else statements - to improve readability I would also rename 'keyStart/End with tokenStart/End Tests are OK.
Created attachment 151909 [details] fix + tests
Comment on attachment 151909 [details] fix + tests That's how I like it!
Deepak, please attach a patch that adds some additional tests which cover the bug made in the previous patch.
Verified for 3.6 M4 with I20091207-1800.
>Deepak, please attach a patch that adds some additional tests which cover the >bug made in the previous patch. This is still missing. I filed bug to track this.
>I filed bug to track this. Bug 297320.
