Permalink
Browse files

Graceful handling of parsing errors from template compiler

* Because the Play template compiler throws an exception as soon as a parsing error is
encountered, calls to the Play template compiler are now wrapped in a ``Try``.
It's the responsability of the calling code to handle the failure case as it
makes sense.

* Use ``Annotation`` for reporting both parsing and compilation errors from
template files. The implication of this is that compilation  errors are no longer
displayed in the Problem View or the Package Explorer.

* Because we now solely use Annotation, we no longer need to use the Eclipse
extension point for defining new markers (look at the deleted lines in
plugin.xml).

* Finally, I'm aware that the code could use some more clean-up, but my intention
was to minimize changes as we plan to release 0.1 possibly next week already.

Fixes #29
  • Loading branch information...
dotta committed Mar 27, 2013
1 parent b3b3270 commit be83b7f38b778e7bbfb28bf2b4f0cc844e0f9de1
@@ -1,13 +1,13 @@
package org.scalaide.play2.templateeditor.lexical
+import scala.tools.eclipse.testsetup.SDTTestUtils
import scala.tools.eclipse.testsetup.TestProjectSetup
-import org.junit.Test
-import org.junit.Assert._
-import org.scalaide.play2.templateeditor.TemplateCompilationUnit
+
import org.eclipse.jdt.internal.core.util.SimpleDocument
-import scala.tools.eclipse.testsetup.FileUtils
import org.junit.AfterClass
-import scala.tools.eclipse.testsetup.SDTTestUtils
+import org.junit.Assert._
+import org.junit.Test
+import org.scalaide.play2.templateeditor.TemplateCompilationUnit
object TemplateCompilationUnitTest extends TestProjectSetup("aProject", bundleName = "org.scala-ide.play2.tests") {
@AfterClass
@@ -35,23 +35,36 @@ class TemplateCompilationUnitTest {
val document = new TestDocument(content1)
templateCU.connect(document)
- val generated1 = templateCU.generatedSource.content
+ val generated1 = templateCU.generatedSource.get.content
val content2 = "@(messages: String)\n<html><body>@messages</body></html>\n"
document.set(content2)
- val generated2 = templateCU.generatedSource.content
+ val generated2 = templateCU.generatedSource.get.content
assertFalse("generated1 and generated2 should be different", generated1 == generated2)
document.set(content1)
- val generated1_2 = templateCU.generatedSource.content
+ val generated1_2 = templateCU.generatedSource.get.content
assertEquals("Same content should return the same generated code", generated1, generated1_2)
}
+ @Test
+ def no_scala_source_is_generated_when_there_are_template_parse_errors() {
+ val tFile = file("app/views/template_parse_error.scala.html")
+ val tu = TemplateCompilationUnit(tFile)
+ assertTrue(tu.generatedSource.isFailure)
+ }
+
+ @Test
+ def scala_source_is_generated_when_there_are_scala_compiler__errors() {
+ val tFile = file("app/views/scala_compiler_error.scala.html")
+ val tu = TemplateCompilationUnit(tFile)
+ assertTrue(tu.generatedSource.isSuccess)
+ }
}
/**
@@ -0,0 +1,5 @@
+@(message: String)
+
+@main("Welcome") {
+ <p>@foo</p>
+}
@@ -0,0 +1,5 @@
+@(message: String)
+
+@main("Welcome") {
+ <p>@</p>
+}
@@ -72,41 +72,6 @@
class="org.scalaide.play2.templateeditor.properties.TemplateColourPreferenceInitializer">
</initializer>
</extension>
- <extension
- point="org.eclipse.ui.editors.annotationTypes">
- <type
- markerSeverity="2"
- markerType="org.scala-ide.play2.templateProblem"
- name="org.scala-ide.play2.error"
- super="org.eclipse.ui.workbench.texteditor.error">
- </type>
- <type
- markerSeverity="1"
- markerType="org.scala-ide.play2.templateProblem"
- name="org.scala-ide.play2.warning"
- super="org.eclipse.ui.workbench.texteditor.warning">
- </type>
- <type
- markerSeverity="0"
- markerType="org.scala-ide.play2.templateProblem"
- name="org.scala-ide.play2.info"
- super="org.eclipse.ui.workbench.texteditor.info">
- </type>
- </extension>
- <extension
- id="templateProblem"
- name="Template Problem"
- point="org.eclipse.core.resources.markers">
- <super
- type="org.eclipse.core.resources.problemmarker">
- </super>
- <super
- type="org.eclipse.core.resources.textmarker">
- </super>
- <persistent
- value="false">
- </persistent>
- </extension>
<extension
point="org.eclipse.core.contenttype.contentTypes">
<content-type
@@ -10,7 +10,6 @@ object PlayPlugin {
@volatile var plugin: PlayPlugin = _
private final val PluginId = "org.scala-ide.play2"
- final val ProblemMarkerId = PluginId + ".templateProblem"
final val RouteFormatterMarginId = PluginId + ".routeeditor.margin"
final val TemplateExtension = "scala.html"
@@ -24,14 +23,14 @@ object PlayPlugin {
class PlayPlugin extends AbstractUIPlugin {
import PlayPlugin._
override def start(context: BundleContext) = {
- super.start(context);
- PlayPlugin.plugin = this;
+ super.start(context)
+ PlayPlugin.plugin = this
initializeProjects()
}
override def stop(context: BundleContext) = {
- PlayPlugin.plugin = null;
- super.stop(context);
+ PlayPlugin.plugin = null
+ super.stop(context)
}
def asPlayProject(project: IProject): Option[PlayProject] = {
@@ -16,7 +16,7 @@ class PlayProject private (val scalaProject: ScalaProject) {
op(presentationCompiler)
}
- def withSourceFile[T](tcu: TemplateCompilationUnit)(op: (SourceFile, ScalaPresentationCompiler) => T): T = {
+ def withSourceFile[T](tcu: TemplateCompilationUnit)(op: (SourceFile, ScalaPresentationCompiler) => T): Option[T] = {
withPresentationCompiler { compiler =>
compiler.withSourceFile(tcu)(op)
}
@@ -1,40 +1,38 @@
package org.scalaide.play2.templateeditor
-import java.io.File
+import java.io.PrintStream
+
import scala.tools.eclipse.InteractiveCompilationUnit
import scala.tools.eclipse.ScalaPlugin
import scala.tools.eclipse.ScalaPresentationCompiler
-import scala.tools.eclipse.resources.MarkerFactory
-import scala.tools.eclipse.util.EclipseFile
+import scala.tools.eclipse.logging.HasLogger
import scala.tools.eclipse.util.EclipseResource
import scala.tools.nsc.interactive.Response
import scala.tools.nsc.io.AbstractFile
+import scala.tools.nsc.io.VirtualFile
import scala.tools.nsc.util.BatchSourceFile
import scala.tools.nsc.util.SourceFile
+import scala.util.Try
+
import org.eclipse.core.resources.IFile
-import org.eclipse.core.resources.IMarker
-import org.eclipse.core.resources.IResource
import org.eclipse.jdt.core.compiler.IProblem
import org.eclipse.jface.text.IDocument
-import org.eclipse.jface.text.Position
+import org.eclipse.jface.text.IRegion
import org.eclipse.jface.text.Region
import org.eclipse.ui.IEditorInput
import org.eclipse.ui.part.FileEditorInput
-import org.eclipse.ui.texteditor.ITextEditor
import org.scalaide.play2.PlayPlugin
import org.scalaide.play2.PlayProject
-import org.scalaide.play2.templateeditor.compiler.PositionHelper
-import scala.tools.nsc.io.VirtualFile
-import java.io.PrintStream
-import org.eclipse.jface.text.IRegion
import org.scalaide.play2.templateeditor.compiler.CompilerUsing
+import org.scalaide.play2.templateeditor.compiler.PositionHelper
+
import play.templates.GeneratedSourceVirtual
/** A Template compilation unit connects the presentation compiler
* view of a tmeplate with the Eclipse IDE view of the underlying
* resource.
*/
-case class TemplateCompilationUnit(val workspaceFile: IFile) extends InteractiveCompilationUnit {
+case class TemplateCompilationUnit(val workspaceFile: IFile) extends InteractiveCompilationUnit with HasLogger {
private var document: Option[IDocument] = None
@@ -71,7 +69,7 @@ case class TemplateCompilationUnit(val workspaceFile: IFile) extends Interactive
override def getContents: Array[Char] = {
withSourceFile({ (sourceFile, compiler) =>
sourceFile.content
- })()
+ })(null)
}
/** Return contents of template file
@@ -116,61 +114,46 @@ case class TemplateCompilationUnit(val workspaceFile: IFile) extends Interactive
}
override def withSourceFile[T](op: (SourceFile, ScalaPresentationCompiler) => T)(orElse: => T = scalaProject.defaultOrElse): T = {
- playProject.withSourceFile(this)(op)
- }
-
- def clearBuildErrors(): Unit = {
- workspaceFile.deleteMarkers(PlayPlugin.ProblemMarkerId, true, IResource.DEPTH_INFINITE)
- }
-
- def reportBuildError(errorMsg: String, start: Int, end: Int, line: Int): Unit = {
- reportBuildError(errorMsg, new Position(start, end - start + 1), line)
- }
-
- def reportBuildError(errorMsg: String, position: Position, line: Int): Unit = {
- def positionConvertor(position: Position, line: Int) = {
- MarkerFactory.RegionPosition(position.offset, position.length, line)
- }
- val pos = positionConvertor(position, line)
- TemplateProblemMarker.create(workspaceFile, IMarker.SEVERITY_ERROR, errorMsg, pos)
+ playProject.withSourceFile(this)(op) getOrElse (orElse)
}
/** maps a region in template file into generated scala file
*/
- def mapTemplateToScalaRegion(region: IRegion) = {
- synchronized {
- val offset = mapTemplateToScalaOffset(region.getOffset())
- val end = mapTemplateToScalaOffset(region.getOffset() + region.getLength() - 1)
- new Region(offset, end - offset + 1)
- }
+ def mapTemplateToScalaRegion(region: IRegion): Option[IRegion] = synchronized {
+ for {
+ start <- mapTemplateToScalaOffset(region.getOffset())
+ end <- mapTemplateToScalaOffset(region.getOffset() + region.getLength() - 1)
+ } yield new Region(start, end - start + 1)
}
/** maps an offset in template file into generated scala file
*/
- def mapTemplateToScalaOffset(offset: Int) = {
- playProject.withPresentationCompiler { pc =>
- val gen = generatedSource()
- PositionHelper.mapSourcePosition(gen.matrix, offset)
+ def mapTemplateToScalaOffset(offset: Int): Option[Int] = synchronized {
+ for(genSource <- generatedSource().toOption) yield {
+ playProject.withPresentationCompiler { pc =>
+ PositionHelper.mapSourcePosition(genSource.matrix, offset)
+ }
}
}
/** Return the offset in the template file, given an offset in the generated source file.
* It is the inverse of `mapTemplateToScalaOffset`. */
- def templateOffset(generatedOffset: Int): Int = {
- generatedSource().mapPosition(generatedOffset)
+ def templateOffset(generatedOffset: Int): Option[Int] = synchronized {
+ generatedSource().toOption.map(_.mapPosition(generatedOffset))
}
-
- private var cachedGenerated = generatedSource()
+ /* guarded by `this`*/
+ private var cachedGenerated: Try[GeneratedSourceVirtual] = generatedSource()
+ /* guarded by `this`*/
private var oldContents = getTemplateContents
/** Returns generated source of the given compilation unit.
*
* It caches results in order to save on (relatively expensive) calls to the template compiler.
*/
- def generatedSource(): GeneratedSourceVirtual = {
- if (oldContents != getTemplateContents) synchronized {
+ def generatedSource(): Try[GeneratedSourceVirtual] = synchronized {
+ if (oldContents != getTemplateContents) {
oldContents = getTemplateContents
- println("[generating template] " + getTemplateFullPath)
+ logger.debug("[generating template] " + getTemplateFullPath)
cachedGenerated = CompilerUsing.compileTemplateToScalaVirtual(getTemplateContents.toString(), file.file, playProject)
}
cachedGenerated
@@ -184,8 +167,6 @@ case class TemplateCompilationUnit(val workspaceFile: IFile) extends Interactive
}
-object TemplateProblemMarker extends MarkerFactory(PlayPlugin.ProblemMarkerId)
-
object TemplateCompilationUnit {
private def fromEditorInput(editorInput: IEditorInput): TemplateCompilationUnit = TemplateCompilationUnit(getFile(editorInput))
@@ -1,32 +1,24 @@
package org.scalaide.play2.templateeditor
+import scala.collection.JavaConverters
+import scala.tools.eclipse.ISourceViewerEditor
+import scala.tools.eclipse.InteractiveCompilationUnit
+import scala.tools.eclipse.ui.InteractiveCompilationUnitEditor
import scala.tools.eclipse.util.SWTUtils.fnToPropertyChangeListener
-import org.eclipse.jdt.internal.ui.text.java.hover.SourceViewerInformationControl
-import org.eclipse.jface.preference.IPreferenceStore
-import org.eclipse.jface.text.IInformationControlCreator
-import org.eclipse.jface.text.source.IOverviewRuler
-import org.eclipse.jface.text.source.IVerticalRuler
-import org.eclipse.jface.text.source.projection.ProjectionSupport
-import org.eclipse.jface.text.source.projection.ProjectionViewer
+
+import org.eclipse.jdt.core.compiler.IProblem
+import org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitDocumentProvider.ProblemAnnotation
+import org.eclipse.jface.text.Position
+import org.eclipse.jface.text.source.IAnnotationModel
+import org.eclipse.jface.text.source.IAnnotationModelExtension
+import org.eclipse.jface.text.source.IAnnotationModelExtension2
+import org.eclipse.jface.text.source.ISourceViewer
import org.eclipse.jface.util.IPropertyChangeListener
import org.eclipse.jface.util.PropertyChangeEvent
-import org.eclipse.swt.SWT
-import org.eclipse.swt.layout.FillLayout
-import org.eclipse.swt.layout.GridData
-import org.eclipse.swt.layout.GridLayout
-import org.eclipse.swt.widgets.Composite
-import org.eclipse.swt.widgets.Shell
import org.eclipse.ui.editors.text.EditorsUI
import org.eclipse.ui.editors.text.TextEditor
-import org.eclipse.ui.texteditor.AnnotationPreference
-import org.eclipse.ui.texteditor.MarkerAnnotationPreferences
-import org.scalaide.play2.PlayPlugin
-import scala.tools.eclipse.ISourceViewerEditor
-import org.eclipse.jface.text.source.ISourceViewer
import org.eclipse.ui.texteditor.ChainedPreferenceStore
-import org.eclipse.jdt.internal.ui.JavaPlugin
-import scala.tools.eclipse.ui.InteractiveCompilationUnitEditor
-import scala.tools.eclipse.InteractiveCompilationUnit
+import org.scalaide.play2.PlayPlugin
class TemplateEditor extends TextEditor with ISourceViewerEditor with InteractiveCompilationUnitEditor {
private lazy val preferenceStore = new ChainedPreferenceStore(Array((EditorsUI.getPreferenceStore()), PlayPlugin.prefStore))
@@ -38,7 +30,7 @@ class TemplateEditor extends TextEditor with ISourceViewerEditor with Interactiv
setDocumentProvider(documentProvider);
override def dispose() = {
- super.dispose();
+ super.dispose()
PlayPlugin.prefStore.removePropertyChangeListener(preferenceListener)
}
@@ -59,4 +51,23 @@ class TemplateEditor extends TextEditor with ISourceViewerEditor with Interactiv
override def getViewer: ISourceViewer = getSourceViewer
override def getInteractiveCompilationUnit(): InteractiveCompilationUnit = TemplateCompilationUnit.fromEditor(this)
+
+ @volatile
+ private var previousAnnotations: List[ProblemAnnotation] = Nil
+
+ private type IAnnotationModelExtended = IAnnotationModel with IAnnotationModelExtension with IAnnotationModelExtension2
+
+ /** Return the annotation model associated with the current document. */
+ private def annotationModel: IAnnotationModelExtended = getDocumentProvider.getAnnotationModel(getEditorInput).asInstanceOf[IAnnotationModelExtended]
+
+ def updateErrorAnnotations(errors: List[IProblem]) {
+ import scala.collection.JavaConverters._
+
+ def position(p: IProblem) = new Position(p.getSourceStart, p.getSourceEnd - p.getSourceStart + 1)
+
+ val newAnnotations = for (e <- errors) yield { (new ProblemAnnotation(e, null), position(e)) }
+
+ annotationModel.replaceAnnotations(previousAnnotations.toArray, newAnnotations.toMap.asJava)
+ previousAnnotations = newAnnotations.unzip._1
+ }
}
Oops, something went wrong.

0 comments on commit be83b7f

Please sign in to comment.