Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XML mode Scala escapes should consider implicit conversion to NodeSeq #3573

Closed
scabug opened this issue Jun 16, 2010 · 6 comments
Closed

XML mode Scala escapes should consider implicit conversion to NodeSeq #3573

scabug opened this issue Jun 16, 2010 · 6 comments

Comments

@scabug
Copy link

scabug commented Jun 16, 2010

I have a class that I can implicitly convert to NodeSeq. It looks something like this:

import scala.xml._

class Xmlable(val i: Int)

object Xmlable {
  implicit def xmlableToNodeSeq(x: Xmlable): NodeSeq = Text("i = " + x.i)
}

I'm surprised that when I use an instance of Xmltable this class in a XML-mode Scala escape expression (not sure exactly what the name for it is), Scala does not invoke the implicit conversion. It seems like it would make sense for Scala to attempt to convert the escape expression to a NodeSeq and only fall back on a default conversion (to Atom or Text via toString) if necessary.

Strangely Scala does use the implicit conversion if I use an Xmlable as an attribute.

scala> import scala.xml._    
import scala.xml._

scala> val x = new Xmlable(1)
x: Xmlable = Xmlable@de35b38

scala> val ns: NodeSeq = x // Implicit conversion used  
ns: scala.xml.NodeSeq = i = 1

scala> <sometag attr={x}/> // Implicit conversion used
res0: scala.xml.Elem = <sometag attr="i = 1"></sometag>

scala> <sometag>{x}</sometag> // Implicit conversion NOT used
res1: scala.xml.Elem = <sometag>Xmlable@de35b38</sometag>
@scabug
Copy link
Author

scabug commented Jun 16, 2010

Imported From: https://issues.scala-lang.org/browse/SI-3573?orig=1
Reporter: Willis Blackburn (willisblackburn)
See #1787
Attachments:

@scabug
Copy link
Author

scabug commented Jun 16, 2010

@harrah said:
Not in the official specification, but according to the unofficial one:

The first translates to

  new UnprefixedAttribute("attr", x, xml.Null)

while the second translates to

  val buffer = new xml.NodeBuffer
  buffer &+ x
  new xml.Elem(null, "sometag", xml.Null, scope, buffer)

!UnprefixedAttribute has overloaded constructors:

    new UnprefixedAttribute(key: String, value: String, next: MetaData)
    new UnprefixedAttribute(key: String, value: Seq[Node], next1: MetaData)
    new UnprefixedAttribute(key: String, value: Option[Seq[Node]], next: MetaData)

The signature of &+ is:

   &+(o: Any): NodeBuffer

So, an attribute value must conform to String, Seq[Node], or Option[Seq[Node]]. Element content is allowed to be anything and so implicits are never considered.

This is also why

  <sometag attr={1}/>

doesn't work, but this does:

  <sometag>{1}</sometag>

In any case, this is essentially the same issue as #1787 and should probably be closed as a duplicate.

@scabug
Copy link
Author

scabug commented Jun 16, 2010

@harrah said:
Well, perhaps it is different enough to be a separate ticket. With the attached patch:

scala> <sometag>{3}</sometag>   
res1: scala.xml.Elem = <sometag>3</sometag>

scala> <sometag>{new Xmlable(3)}</sometag>
res2: scala.xml.Elem = <sometag>i = 3</sometag>

In theory it should be source compatible (test suite passes), but I'm sure it breaks someone's code somewhere.

#1787 requires a more invasive approach since type parameters aren't allowed on auxiliary constructors.

@scabug
Copy link
Author

scabug commented Jun 16, 2010

Willis Blackburn (willisblackburn) said:
The patch is very elegant! I've learned a couple of things about Scala just by looking at it. For some reason it never occurred to me that I could default an implicit parameter.

I'd be surprised if there are too much existing code where (a) some class X is defined, (b) there's an implicit conversion from X to NodeSeq in scope, and (c) an expression evaluating to X is used within {}, but (d) the developer expects the X to be converted to NodeSeq via toString.

One suggestion: If the method is called with an Iterable[T] and there is an implicit conversion from T to NodeSeq in scope, then IMHO the method should map the Iterable with the conversion function before passing it to addAny.

@scabug
Copy link
Author

scabug commented Jun 16, 2010

@harrah said:
The signature is the right one, in my opinion. You could add an overload on Seq[T], but you can't have overloaded methods with defaults and even if you could, you are restricting yourself to one level of nesting.

Rather, the caller could define an implicit like:

implicit def views[T](x: Seq[T])(implicit view: T => Seq[Node]): Seq[Node] =
  x flatMap view

With that you can nest arbitrarily:

scala> val xs = Seq(3,4) map (new Xmlable(_))
xs: Seq[Xmlable] = List(Xmlable@6d172f8f, Xmlable@d338d3d)
                                
scala <x>{Seq(xs,xs)}</x>
res4: scala.xml.Elem = <x>i = 3i = 4i = 3i = 4</x>

@scabug
Copy link
Author

scabug commented Jul 17, 2015

@SethTisue said:
The scala-xml library is now community-maintained. Issues with it are now tracked at https://github.com/scala/scala-xml/issues instead of here in the Scala JIRA.

Interested community members: if you consider this issue significant, feel free to open a new issue for it on GitHub, with links in both directions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant