Community
Participate
Working Groups
Build ID: M20080911-1700 BatchMessagerImpl and IdeMessagerImpl are currently ignoring AnnotationMirror and AnnotationValue parameters passed to Messager#printMessage(Kind, CharSequence, Element, AnnotationMirror, AnnotationValue). This makes APT problem reporting less precise.
Created attachment 123054 [details] A patch for MessagerTests Walter, I've found that MessagerProc really tries to report errors on the annotation @AnnoZ on the class D and its single annotation value, but MessagerTests expects output from the Eclipse compiler that ignores that additional context and reports errors on just the class D itself (see ERROR 3 and ERROR 4 in EXPECTED_ECLIPSE_MESSAGES). So it seems to me that currently MessagerTests pass but they should not. Given the nature of MessagerTests however, it is difficult to make a failing test before the issue is fixed because it is not known exactly what compiler output to expect. For reference, I've made a patch demonstrating corresponding javac output.
Well, up until very recently the MessagerTests didn't actually check the content of the message at all, I just added that in order to check the output of the last part of this that you and I worked on. The JSR269 spec leaves it up to the implementation as to how much of this detail is honored and what the format is. About the only thing you can count on is that the Eclipse compiler will *not* produce the same output format as javac... makes it hard to write a good test. Anyway, as you observe, the problem is that BaseMessagerImpl.createProblem() does not have a way of dealing with anything smaller than an element. But I don't remember why it is that way - it might be a compiler limitation, or it might just be that we were short on time. Want to try your hand at a patch for that? Oh, and no need to add me as a cc - I get everything that goes to jdt-apt-inbox. I just didn't have anything cogent to say yet :-)
Sorry for the CC, Walter. Got it. When I have a little more time, I will definitely look into this and do my best to make a patch. Thank you!
Created attachment 125636 [details] Proposed patch
Hi Vladimir, That looks pretty good to me, although I need to spend a bit more time reviewing it. In order to commit it, I need to pass it through the Eclipse IP process (http://www.eclipse.org/legal/EclipseLegalProcessPoster.pdf) which requires me to ask you: 1. Did you write 100% of this patch yourself? 2. Do you have the right to contribute it to Eclipse? 3. I see that the file header contains the EPL license header as required, but you've assigned the copyright and initial authorship to IBM Corporation. Is that intentional (i.e. do you work for IBM), or should it be something else? If you're already an Eclipse committer (on some other Eclipse component) then just let me know that and we can ignore all of this. Thanks!
Hi Walter, No, I'm not an Eclipse commiter. So here are the answers: 1. Yes. 2. Yes. 3. No, I don't work for IBM. I thought the header was standard and just copy-pasted it. It is my first time contributing a patch containing new classes, so it is easy to be mistaken, sorry. Should I now provide the patch with the copyright fixed? Can you provide some guidelines (or link at the guidelines) on how the EPL header should look like? Thank you!
(In reply to comment #6) > 3. No, I don't work for IBM. I thought the header was standard and just > copy-pasted it. It is my first time contributing a patch containing new > classes, so it is easy to be mistaken, sorry. Should I now provide the patch > with the copyright fixed? Can you provide some guidelines (or link at the > guidelines) on how the EPL header should look like? What you have is fine, just change the copyright and author, like: /******************************************************************************* * Copyright (c) 2009 Vladimir Piskarev and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at * http://www.eclipse.org/legal/epl-v10.html * * Contributors: * Vladimir Piskarev - initial API and implementation *******************************************************************************/ Or if you need/want to assign copyright to some other entity, change accordingly. You can replace the old patch with a new one, just mark the old one as obsolete.
Created attachment 125692 [details] New proposed patch
Committed Vladimir's fix, and updated MessagerTest accordingly. The contribution is much appreciated!
