23

I need to change the signature of a method used all over the codebase.

Specifically, the method void log(String) will take two additional arguments (Class c, String methodName), which need to be provided by the caller, depending on the method where it is called. I can't simply pass null or similar.

To give an idea of the scope, Eclipse found 7000 references to that method, so if I change it the whole project will go down. It will take weeks for me to fix it manually.

As far as I can tell Eclipse's refactoring plugin of Eclipse is not up to the task, but I really want to automate it.
So, how can I get the job done?

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
Hayati Guvence
  • 718
  • 2
  • 6
  • 22
  • 16
    Those who do not learn from log4j are condemned to repeat it, badly. – Paul Tomblin Oct 11 '10 at 12:36
  • Unfortunately, I wasn't the one who wrote the framework. However, I am the one who is condemned. – Hayati Guvence Oct 11 '10 at 12:46
  • @Hayati Have you considered writing your own small program to do the replacement? – Petar Minchev Oct 11 '10 at 13:00
  • Its an interesting academic question. However, the specific case you give is horrible. Is every line of logging code expected to express its class and its method name? Really? And who says using the stack is too slow? Did anyone profile that, or is that pure assumption? – Yishai Oct 11 '10 at 13:02
  • 1
    Consider - your plan recognizes the need for refactoring, but will result in stringified method names that will not be included in future automated refactoring. – Andy Thomas Oct 11 '10 at 13:24
  • 2
    Guys, from what I hear: it's not his fault, he has to clean up the mess after others. Boy, have I been there. Haven't we all? – Sean Patrick Floyd Oct 11 '10 at 13:33
  • 2
    An existing mess does not merit creation of a larger mess. – Andy Thomas Oct 11 '10 at 13:58
  • @Andy True, but it doesn't sound like it's his choice to make. – Sean Patrick Floyd Oct 11 '10 at 14:15
  • @Petar Yes i did. I realized the consequences of parsing by myself will cause me to write a java compiler from the scratch. Using regex was gonna cause tuning up, at the end i will encounter a problem meaning dead end. Instead, I wanted to look for a tool, a language, a library or something which can save my time. – Hayati Guvence Oct 11 '10 at 14:24
  • 2
    You don't want to do this twice - have a look at http://www.slf4j.org/ – Pablojim Oct 11 '10 at 18:24
  • You may want to write a tool to do this for you... – Rich Oct 11 '10 at 13:14

12 Answers12

22

Great, I can copy a previous answer of mine and I just need to edit a tiny little bit:


I think what you need to do is use a source code parser like javaparser to do this.

For every java source file, parse it to a CompilationUnit, create a Visitor, probably using ModifierVisitor as base class, and override (at least) visit(MethodCallExpr, arg). Then write the changed CompilationUnit to a new File and do a diff afterwards.

I would advise against changing the original source file, but creating a shadow file tree may me a good idea (e.g. old file: src/main/java/com/mycompany/MyClass.java, new file src/main/refactored/com/mycompany/MyClass.java, that way you can diff the entire directories).

Sean Patrick Floyd
  • 292,901
  • 67
  • 465
  • 588
6

Eclipse is able to do that using Refactor -> Change Method signature and provide default values for the new parameters.

For the class parameter the defaultValue should be this.getClass() but you are right in your comment I don't know how to do for the method name parameter.

Manuel Selva
  • 18,554
  • 22
  • 89
  • 134
  • Default values can vary depending on its place from where `void log` method is being invoked. I need something more dynamic. – Hayati Guvence Oct 11 '10 at 13:03
  • @Hayati si my edited answer. I asked for help here: http://manuelselva.wordpress.com/2010/10/11/eclipse-method-refactoring/ I'll keep you inform about answers – Manuel Selva Oct 11 '10 at 13:32
3

IntelliJ IDEA shouldn't have any trouble with this.

I'm not a Java expert, but something like this could work. It's not a perfect solution (it may even be a very bad solution), but it could get you started:

Change the method signature with IntelliJ's refactoring tools, and specify default values for the 2 new parameters:

c: self.getClass()
methodName: Thread.currentThread().getStackTrace()[1].getMethodName()

or better yet, simply specify null as the default values.

Philippe Leybaert
  • 168,566
  • 31
  • 210
  • 223
2

I think that there are several steps to dealing with this, as it is not just a technical issue but a 'situation':

  1. Decline to do it in short order due to the risk.
  2. Point out the issues caused by not using standard frameworks but reinventing the wheel (as Paul says).
  3. Insist on using Log4j or equivalent if making the change.
  4. Use Eclipse refactoring in sensible chunks to make the changes and deal with the varying defaults.

I have used Eclipse refactoring on quite large changes for fixing old smelly code - nowadays it is fairly robust.

Jason
  • 915
  • 6
  • 6
  • 2
    Yes, this answer makes sense, but no, I wouldn't recommend it. If the manager says it needs to be done then most of the time you're going to have to do it, whether it's smart or not. And the only remaining question is the one he's asking here: what is the quickest reliable way of doing this. Being right doesn't always let you keep your job. – Sean Patrick Floyd Oct 11 '10 at 13:37
  • 2
    There is a middle ground between the two extremes of vocal refusal and silent obedience. Take five minutes to share concerns with your supervisor. – Andy Thomas Oct 11 '10 at 18:03
2

Maybe I'm being naive, but why can't you just overload the method name?

void thing(paramA) {
    thing(paramA, THE_DEFAULT_B, THE_DEFAULT_C)
}

void thing(paramA, paramB, paramC) {
    // new method
}
Amy B
  • 17,874
  • 12
  • 64
  • 83
  • The problem is that the defaults aren't defaults:) He passes the name of the class and the name of the method. They vary... – Petar Minchev Oct 11 '10 at 13:46
  • If it's acceptable to add the new parameters for a subset of calls and get to others as you get to them, this is a good answer. Often when I have to change code that's called from many places, I use a solution like this to minimize the regression impact. – Jay Oct 11 '10 at 13:50
2

Do you really need to change the calling code and the method signature? What I'm getting at is it looks like the added parameters are meant to give you the calling class and method to add to your log data. If the only requirement is just adding the calling class/method to the log data then Thread.currentThread().getStackTrace() should work. Once you have the StackTraceElement[] you can get the class name and method name for the caller.

  • 3
    True. That's the smartest thing to do. However, Using StrackTrace or Throwable object decreases the performance badly if its being called from 7000 places by enormous number of online users. Thanks for your comment though. – Hayati Guvence Oct 11 '10 at 15:33
  • @Hayati Really, do you think the performance hit is comparable to the I/O of the logging, assuming you're logging to disk? – Troy Bourdon Oct 11 '10 at 15:46
  • You could delay the evaluation of the stacktrace beyond the log level test (if you are using log levels). – whiskeysierra Oct 11 '10 at 23:01
1

If the lines you need replaced fall into a small number of categories, then what you need is Perl:

find -name '*.java' | xargs perl -pi -e 's/log\(([^,)]*?)\)/log(\1, "foo", "bar")/g'

I'm guessing that it wouldn't be too hard to hack together a script which would put the classname (derived from the filename) in as the second argument. Getting the method name in as the third argument is left as an exercise to the reader.

uckelman
  • 25,298
  • 8
  • 64
  • 82
1

Try refactor using intellij. It has a feature called SSR (Structural Search and Replace). You can refer classes, method names, etc for a context. (seanizer's answer is more promising, I upvoted it)

Jayan
  • 18,003
  • 15
  • 89
  • 143
1

I agree with Seanizer's answer that you want a tool that can parse Java. That's necessary but not sufficient; what you really want is a tool that can carry out a reliable mass-change.

To do this, you want a tool that can parse Java, can pattern match against the parsed code, install the replacement call, and spit out the answer without destroying the rest of the source code.

Our DMS Software Reengineering Toolkit can do all of this for a variety of languages, including Java. It parses complete java systems of source, builds abstract syntax trees (for the entire set of code).

DMS can apply pattern-directed, source-to-source transformations to achieve the desired change.

To achieve the OP's effect, he would apply the following program transformation:

 rule replace_legacy_log(s:STRING): expression -> expression
    " log(\s) " -> " log( \s, \class\(\), \method\(\) ) "

What this rule says is, find a call to log which has a single string argument, and replace it with a call to log with two more arguments determined by auxiliary functions class and method.

These functions determine the containing method name and containing class name for the AST node root where the rule finds a match.

The rule is written in "source form", but actually matches against the AST and replaces found ASTs with the modified AST.

To get back the modified source, you ask DMS to simply prettyprint (to make a nice layout) or fidelity print (if you want the layout of the old code preserved). DMS preserves comments, number radixes, etc.\

If the exisitng application has more than one defintion of the "log" function, you'll need to add a qualifier:

... if IsDesiredLog().

where IsDesiredLog uses DMS's symbol table and inheritance information to determine if the specific log refers to the definition of interest.

Ira Baxter
  • 93,541
  • 22
  • 172
  • 341
0

Il fact your problem is not to use a click'n'play engine that will allow you to replace all occurences of

log("some weird message");

by

log(this.getClass(), new Exception().getStackTrace()[1].getMethodName());

As it has few chances to work on various cases (like static methods, as an example).

I would tend to suggest you to take a look at spoon. This tool allows source code parsing and transformation, allowing you to achieve your operation in a -obviously code based- slow, but controlled operation.

However, you could alos consider transforming your actual method with one exploring stack trace to get information or, even better, internally use log4j and a log formatter that displays the correct information.

Riduidel
  • 22,052
  • 14
  • 85
  • 185
  • Using StrackTrace can affect the performance badly. That's not my option. I am having a look at the tool you suggested. – Hayati Guvence Oct 11 '10 at 12:49
  • Of course it is a bad idea, that's why i suggest you took the spoon route (although I have to notice a good bunch of regexp could do the job for you) – Riduidel Oct 11 '10 at 13:00
  • *a good bunch of regexp could do the job for you* yes, but it could also really mess things up. – Sean Patrick Floyd Oct 11 '10 at 13:28
  • 2
    Oh, you know the magic quote ? "you have a problem ? You use regular expressions ? Now you have *two* problems" – Riduidel Oct 11 '10 at 13:54
  • You can't use regular expressions to reliably parse Java. At least one of your problems is explicit. – Ira Baxter Oct 12 '10 at 07:44
0

I would search and replace log( with log(@class, @methodname,

Then write a little script in any language (even java) to find the class name and the method names and to replace the @class and @method tokens...

Good luck

pgras
  • 12,614
  • 4
  • 38
  • 46
  • If there is more than one "log" function this will destroy the program. If there is only one log function, is that script "little"? It has to understand all the lexical conventions of Java to get it right, and likely a lot of the syntax. Can you make it do 80% with a little script? Perhaps. Would you know for a particular reference that it was right (you might have to check all 7000 references). For those it didn't do right, that would leave some 1400 to do by hand. Is this really a win? Scripts hacks applied to programming langauge code are often failures. – Ira Baxter Oct 13 '10 at 03:42
  • The job has to be done only once, so why not begin with a hack, check the result and decide if it is an option to correct the remaining problems by hand. And there is no need for a complicated script, the class name is the filename (ok not for inner classes) and the method name is most of the time the last 'XXX(*){' you encountered. And finally as long as the class name is OK if one log message looks weird you can find the class and correct afterwards... – pgras Oct 13 '10 at 10:34
0

If the class and method name are required for "where did this log come from?" type data, then another option is to print out a stack trace in your log method. E.g.

public void log(String text)
{
   StringWriter sw = new StringWriter();
   PrintWriter pw = new PrintWriter(sw, true);
   new Throwable.printStackTrace(pw);
   pw.flush();
   sw.flush();
   String stackTraceAsLog = sw.toString();
   //do something with text and stackTraceAsLog
}
Chris Knight
  • 24,333
  • 24
  • 88
  • 134