Bug 195056 - [refactoring scripts] Scriptable Refactoring XML is not human understandable
Summary: [refactoring scripts] Scriptable Refactoring XML is not human understandable
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-07-01 12:42 EDT by Karsten Becker CLA
Modified: 2007-07-02 08:46 EDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Karsten Becker CLA 2007-07-01 12:42:50 EDT
I used the Change signature refactoring to change the method:
AuctioneerReader.readFile(File, IProgressMonitor)
to:
AuctioneerReader.readFile(File, IProgressMonitor, String, ArrayList)
Strings default was "Hallo" and the default for ArrayList were new ArrayList()

This resulted in the following XML file:
<?xml version="1.0" encoding="utf-8" standalone="no"?>
<session version="1.0">
	<refactoring
		comment="Change method 'private static Section net.kbsvn.auctioneer.io.AuctioneerReader.readFile(File file, IProgressMonitor monitor) throws FileNotFoundException, IOException' to 'private static Section readFile(File file, IProgressMonitor monitor, String test1, util.ArrayList test2) throws FileNotFoundException, IOException'&#13;&#10;- Original project: 'net.kbsvn.auctioneer'&#13;&#10;- Added parameters:&#13;&#10;     String test1&#13;&#10;     java.util.ArrayList test2"
		default3="&quot;Hallo&quot;" default4="new ArrayList()"
		delegate="false" deprecate="true"
		description="Change method 'readFile'"
		element1="/C:\/Programme\/Java\/jdk1.6.0\/jre\/lib\/rt.jar&lt;java.io(FileNotFoundException.class[FileNotFoundException"
		element2="/C:\/Programme\/Java\/jdk1.6.0\/jre\/lib\/rt.jar&lt;java.io(IOException.class[IOException"
		flags="589826" id="org.eclipse.jdt.ui.change.method.signature"
		input="/src&lt;net.kbsvn.auctioneer.io{AuctioneerReader.java[AuctioneerReader~readFile~QFile;~QIProgressMonitor;"
		kind1="0" kind2="0" name="readFile"
		parameter1="File file 0 File file false"
		parameter2="IProgressMonitor monitor 1 IProgressMonitor monitor false"
		parameter3="  -1 String test1 false"
		parameter4="  -1 java.util.ArrayList test2 false"
		project="net.kbsvn.auctioneer" version="1.0" />
</session>
Albeit this xml is well formed it is not possible to see what the user has done by simple looking at this xml. One needs the source code to understand what the kind attribute means and in which order the parameter String is composed. It is sub optimal to include whole pathes to the rt.jar if the script is meant to be cross platform compatible. It would also be not possible to create a schema for this xml to validate it, or its data.
Comment 1 Karsten Becker CLA 2007-07-01 13:02:55 EDT
There are several ways to make the xml more expressive. The most obvious would be to create elements and nested elements. This would make the document better understandable.
<?xml version="1.0" encoding="utf-8" standalone="no"?>
<session version="1.0">
	<refactoring project="net.kbsvn.auctioneer" version="2.0" flags="589826" id="org.eclipse.jdt.ui.change.method.signature">
		<comment>Change method 'private static Section net.kbsvn.auctioneer.io.AuctioneerReader.readFile(File file, IProgressMonitor monitor) throws FileNotFoundException, IOException' to 'private static Section readFile(File file, IProgressMonitor monitor, String test1, util.ArrayList test2) throws FileNotFoundException, IOException'
- Original project: 'net.kbsvn.auctioneer'
- Added parameters:
     String test1
     java.util.ArrayList test2</comment>
		<description>Change method 'readFile'</description>
		<delegates delegate="false" deprecate="true"/>
		<input>net.kbsvn.auctioneer.io.AuctioneerReader.readFile(File, IProgressMonitor)</input>
		<exceptions>
			<exception kind="OLD" class="java.io.FileNotFoundException"/>
			<exception kind="OLD" class="java.io.IOException"/>
		</exceptions> 
		<signature methodName="readFile">
			<parameter oldType="File" oldName="file" oldIndex="0"/><!-- Without newName means not renamed -->
			<parameter oldType="IProgressMonitor" oldName="monitor"  oldIndex="1"/><!-- Without deleted means not deleted -->
			<parameter newType="String" newName="test1" default="&quot;Hallo&quot;"/><!-- No oldName means added -->
			<parameter newType="java.util.ArrayList" newName="test2" default="new ArrayList()"/><!-- Default only valid for added -->
		</signature>
	</refactoring>
</session>
Comment 2 Karsten Becker CLA 2007-07-01 13:11:05 EDT
This still has the drawBack of not being verifable. The solution would be to use namespaces. This also allows for some very nice features like embedded xhtml for the comment.
<?xml version="1.0" encoding="utf-8" standalone="no"?>
<ref:session version="1.0" xmlns="http://www.w3.org/TR/html4/" 
	xmlns:ref="http://www.eclipse.org/refactorings" 
	xmlns:csr="http://www.eclipse.org/refactorings/org.eclipse.jdt.ui.change.method.signature"/>
	<ref:refactoring project="net.kbsvn.auctioneer" version="2.0" flags="589826">
		<ref:comment>Change method 'private static Section net.kbsvn.auctioneer.io.AuctioneerReader.readFile(File file, IProgressMonitor monitor) throws FileNotFoundException, IOException' to 'private static Section readFile(File file, IProgressMonitor monitor, String test1, util.ArrayList test2) throws FileNotFoundException, IOException'<br/>
- Original project: <b>'net.kbsvn.auctioneer'</b><br/>
- Added parameters:<br/>
<ul>
     <li>String test1</li>
     <li>java.util.ArrayList test2</li>
 </ul></ref:comment>
     
		<ref:description>Change method 'readFile'</ref:description>
		<csr:delegates delegate="false" deprecate="true"/>
		<ref:input>net.kbsvn.auctioneer.io.AuctioneerReader.readFile(File, IProgressMonitor)</ref:input>
		<csr:exceptions>
			<csr:exception kind="OLD" class="java.io.FileNotFoundException"/>
			<csr:exception kind="OLD" class="java.io.IOException"/>
		</csr:exceptions> 
		<csr:signature methodName="readFile">
			<csr:parameter oldType="File" oldName="file" oldIndex="0"/><!-- Without newName means not renamed -->
			<csr:parameter oldType="IProgressMonitor" oldName="monitor"  oldIndex="1"/><!-- Without deleted means not deleted -->
			<csr:parameter newType="String" newName="test1" default="&quot;Hallo&quot;"/><!-- No oldName means added -->
			<csr:parameter newType="java.util.ArrayList" newName="test2" default="new ArrayList()"/><!-- Default only valid for added -->
		</csr:signature>
	</ref:refactoring>
</ref:session>
Comment 3 Karsten Becker CLA 2007-07-01 13:15:55 EDT
This has the advantage that every refactoring can have its own namespace and have its own expressivness as required while still keeping the idea of being validatable. It is also easy to builtin other namespaces like the xhtml namespace which allows better control over the formatting of comments i.e.
The namespace is built by prepending the refactoring ID with http://www.eclipse.org/refactorings/ this ensures that every refactoring has a unique namespace and the id does not need to be stored spererately. 
Comment 4 Martin Aeschlimann CLA 2007-07-02 08:46:59 EDT
I agree and I wished we would have gone that way from the beginning. At the moment we don't have the time budget to rewrite this and can only do this if it is required to fix a bug.