Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 354374 - [Issues] DiagnosticConverterImpl.resolveStructuralFeature() should probably resolve?
Summary: [Issues] DiagnosticConverterImpl.resolveStructuralFeature() should probably r...
Status: CLOSED WONTFIX
Alias: None
Product: TMF
Classification: Modeling
Component: Xtext (show other bugs)
Version: 2.0.0   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-10 09:10 EDT by Stephan Herrmann CLA
Modified: 2017-09-19 17:45 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2011-08-10 09:10:43 EDT
We have Check rules that define a context like so

  context MyType#isFeature

but when a diagnostic is created from that rule the whole MyType element
is highlighted, which is undesirable and seems to contradict the intention
behind the context specification.

From debugging it seems that the implementation of 
org.eclipse.xtext.validation.DiagnosticConverterImpl.resolveStructuralFeature(EObject, Object)
is simply lacking s.t. like

   if (feature instanceof String)
       return ele.eClass().getEStructuralFeature((String)feature);

In my experiments in the debugger this produced the exact expected result.

BTW: the migration guides in the user doc seem to lack any mentioning
of Check based validation, but I may have been looking in the wrong
places as I didn't find any reference to (new) extension points,
like org.eclipse.xtend.typesystem.emf.checks.
Comment 1 Sebastian Zarnekow CLA 2011-08-16 17:10:06 EDT
Since the old check language is deprecated, the contract of Diagnostic#getData does not describe Strings as valid entries in the data list and a custom implementation of the DiagnosticConverter serves as a nice workaround, I close this as won't fix. Please reopen, if I missed something.
Comment 2 Stephan Herrmann CLA 2011-08-18 08:33:56 EDT
(In reply to comment #1)
> Since the old check language is deprecated, 

Deprecated as in: you personally no longer recommend its use, or deprecated
as in: all documentation has been updated to alert the user that this 
language will be discontinued, and providing help for migrating to a
suitable replacement?

I'm asking because I see neither the alert nor the migration hints.

> the contract of Diagnostic#getData
> does not describe Strings as valid entries in the data list

It says "arbitrary associated list of data", which is not very restrictive..
OTOH, a method called "resolveStructuralFeature" which does nothing but
a guarded cast looks like some kind of bug to me.

> and a custom
> implementation of the DiagnosticConverter serves as a nice workaround,

I disagree, the implementation of a custom DiagnosticConverter might be
simple, but without internal knowledge it is *not* trivial to setup the
configuration to use the new class.

> I close
> this as won't fix. Please reopen, if I missed something.

I won't fight for it, because I'm already using the solution outlined in
comment 0 (using an OT/Equinox plug-in). I still don't understand why you
don't want it, but that's your choice.
Comment 3 Sven Efftinge CLA 2011-08-18 10:22:59 EDT
Check is slow hard to debug and inflexible in terms of reporting problems and where they were. 
Moreover it drags the runtime dependencies form Xpand in. 
We think even the Java-based API is much nicer, because it is more flexible, easier to debug and has much nicer tooling. However using Xtend 2 is a straight forward alternative for people who dislike the verbose nature of Java. 

You are right in that we missed to list the deprecation of Check in the migration document. It should have been in 0.7 -> 1.0 already.
Comment 4 Karsten Thoms CLA 2017-09-19 17:34:45 EDT
Closing all bugs that were set to RESOLVED before Neon.0
Comment 5 Karsten Thoms CLA 2017-09-19 17:45:53 EDT
Closing all bugs that were set to RESOLVED before Neon.0