Bug 27640 - change return value of FileObject.toString() to cleanly break bad clients
change return value of FileObject.toString() to cleanly break bad clients
Status: VERIFIED FIXED
Product: platform
Classification: Unclassified
Component: Filesystems
3.x
All All
: P3 (vote)
: 4.x
Assigned To: David Konecny
issues@platform
: API
Depends on:
Blocks: 26921
  Show dependency treegraph
 
Reported: 2002-09-27 13:52 UTC by David Strupl
Modified: 2008-12-22 21:39 UTC (History)
5 users (show)

See Also:
Issue Type: TASK
:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Strupl 2002-09-27 13:52:03 UTC
I don't think it makes much sense to have it
deprecated. Either undeprecate it or make this
issue invalid. Please wait until issue 27638 is
solved.
Comment 1 Peter Zavadsky 2002-09-30 14:22:42 UTC
Jesse, if there is no need for the debugging purposes yet. Please
undeprecate and also unfinale the method.
Or reassign it to me and I'll take case about that.
Comment 2 Peter Zavadsky 2002-10-02 10:05:57 UTC
Fixed in [trunk]

openide/../filesystems/FileObject.java 1.59
Comment 3 Jaroslav Tulach 2002-10-02 10:58:15 UTC
Should not apichanges be updated as well, as far as I know the
document contains sentence about making this final + you should update
the build.xml to not patch FileObject and maybe remove the test in
openide/compat (but this need not be necessary).

Comment 4 Peter Zavadsky 2002-10-02 13:11:38 UTC
I added into apichanges just that the javadoc was changed. I didn't
find the words about finalling the methods, just about introduction of
getPath method.
Comment 5 Jesse Glick 2002-10-02 18:34:02 UTC
"I don't think it makes much sense to have it deprecated." - why not?
To date deprecating toString() has been helpful in finding code that
was using it. All code calling toString() needs to be evaluated to see
if it should be using getPath or Classpath API. Not only our code in
openide/core, but other modules, incl. those outside nb.org. If
toString() is left undeprecated, no one will notice that it should be
replaced. I don't think use for debugging purposes is a good excuse -
either call getPath explicitly, or just use

"some message: " + fo

since javac does not report this as a deprecated usage, anyway.

There is no need to update apichanges just for deprecations, I think.
They should be self-explanatory, and you do not need to go looking for
them: just compile your sources.
Comment 6 Peter Zavadsky 2002-10-03 08:33:41 UTC
It was agreed on nbdev, this method shouldn't be deprecated, see
there. It's method defined in Object, with some purpose, and that
purpose can't be changed in subclasses.

If we need to check the former bad usage of that, we should use just
some temp tool for finding it (Hrebejk has written small prog for
finding it, and we've used it to check all the dependecies). Why it
should be deprecated for those programmers who use it correctly?

To the javadoc. Well, I could written it wrongly. All I wanted to say
is, it should be used like ordinary toString method, not misused for
other purposes like we have done in the past, i.e. for determinig full
resource path.
Comment 7 Jesse Glick 2002-10-03 15:34:45 UTC
Yeah, I just found the mailing list thread where this was discussed -
sorry, behind on mails...

Certainly it is defined in Object with a loose meaning - "print some
useful debugging information about object" - and if you are using it
in this way, there is no problem.

However, I deprecated it as a practical matter. A lot of code *was*
using it to get the file path (or, worse, Java resource path), and
that code would have worked without problems for several NB releases.
We even almost recommended doing this in the Javadoc for a while. We
can easily clean up code on netbeans.org (and S1S) by scanning for
usages of FileObject.toString() and replacing them with getPath or
other idioms according to context.

My worry, though, is third-party code. We have no idea who might be
calling FO.tS() and why. If you deprecate the method and provide an
explanation in the Javadoc of why existing uses of it might be wrong,
people who recompile their code are bound to see the message sooner or
later. When you see the message, you can get rid of it by replacing
the call to getPath or something else, as appropriate, so it serves as
a checklist. If we *don't* deprecate it, we can and will

1. Describe all this in the Javadoc, which surely no one will read.

2. Mention it in the upgrade guide. (I think Svata already plans to do
this?)

Let's hope #2 is effective. Naturally module authors will have to be
checking all of their code for various classpath violations in 4.0 -
we should strive to make this as easy as possible. I found deprecation
messages for FO.tS() to be quite convenient in the case of apisupport.

Anyway, I'm not going to push the issue, since I understand the
reasons for not wanting to deprecate an override of a standard method
- just explaining some concerns.
Comment 8 Jaroslav Tulach 2002-10-04 09:50:15 UTC
Unreleated but: Scanning for FileObject.toString is nearly impossible.
 Imagine code:

FileObject fo = ..;
String file = fo.getFileSystem ().getSystemName () + '/' + fo;

I know that this code is bad example, but it also uses toString and
there is no way how to find it out, because it uses

StringBuffer.append (Object)

in the compiled bytecode.
Comment 9 Jesse Glick 2002-10-04 14:49:08 UTC
Yes, I've noticed this, and thought about filing a javac bug about it.
Acc. to JLS 2.0 15.18.1.1: "the conversion is performed as if by an
invocation of the toString method of the referenced object with no
arguments" and later "To increase the performance of repeated string
concatenation, a Java compiler may use the StringBuffer class or a
similar technique" - meaning (to me) that the semantics is defined in
terms of toString(), not StringBuffer.append(Object), and so
deprecations in effect with toString() should be considered, according
to the compile-time type of the object being concatenated.
Comment 10 Peter Zavadsky 2002-10-07 07:31:44 UTC
I think you are still hesitating.
I understand the concerd about third-party users.
Please don't do that.

I think the javadoc is enough.

If nobody read it in the past, then we can be sure nobody uses
toString as a strict formatted output. That usage was approved only in
the javadoc. If nobody read it, nobody misused it.

Well, there could be also another way, that is having seen such usage
in our code. It is no more the case now and it leads to the javadoc
check either.
Anyway I guess this is too obscure thing we should care about.

Comment 11 Jesse Glick 2002-10-07 16:07:22 UTC
"If nobody read it in the past, then we can be sure nobody uses
toString as a strict formatted output. That usage was approved only in
the javadoc. If nobody read it, nobody misused it." - I wish it were
that easy. Unfortunately, whenever you implement a method to do
something that looks like it reliably behaves according to some
specification, even if you do not specify it that way, someone will
start using it that way. For example, a very common error is to assume
that FileSystem.getSystemName on a (local) filesystem will return the
path to the root directory. This is not guaranteed anywhere, in fact
is explicitly not guaranteed by the Javadoc, yet if you try it it
seems to work - and many, many pieces of 3rd-party code I have seen
make this mistake. I have suggested changing the impl to just return
something else so the error would be obvious.

Anyway, I guess Javadoc should be enough - we'll see how many actual
problems arise. I'm sure there will be other common errors that a lot
of people run into when doing classpath conversions, that deprecations
will not/cannot cover.
Comment 12 Jesse Glick 2002-12-09 20:02:38 UTC
Ha, I've got a better idea. Rather than deprecating it, just change it
to cleanly break bad clients.

public String toString() {
    String fsname;
    try {
        fsname = getFileSystem().getSystemName();
    } catch (FileStateInvalidException fsie) {
        fsname = "invalid"; // NOI18N
    }
    return getPath() + "[" + fsname + "]"; // NOI18N
}

Nice and easy and has the desired effect. Modules depending on the
classpath behavior will break nice and quickly for a much more obvious
reason. "Hey! This isn't a resource name! Oh, I see, I meant to use
the ClassPath API / I meant to call getPath()." (Of course Javadoc for
it must point to Classpath API and getPath() with explanation of when
to use which - please fix this too.) Modules which just use it for
debugging won't care much (actually it will be a bit more useful in
most cases).

No one should be relying on its current meaning; see e.g. 3.4 API
Javadoc for it:

---%<---
Get fully-qualified filename. Does so by walking through all folders
to the root of the filesystem. Separates files with provided '/'. The
extension, if present, is separated from the basename with '.'.

Note: fo.getPackageNameExt ('/','.') will have the same effect as
using this method. But it is not guaranteed that this will be so.
---%<---

I.e. it might do something, but then again it might not (depending on
subclasses). Thus if your module depends on it behaving in a certain
way, your module is broken.

I would also like to see getSystemName implemented on various common
filesystems (especially LocalFileSystem) to return something which is
specifically *not* a directory name. This is very often abused.
Comment 13 Jesse Glick 2002-12-09 20:30:13 UTC
BTW Evan I finally see your months-old message "toString (was: Recent
classpath related API deprecations)" on dev@openide, now that the NNTP
archives have been restored (sigh).

---%<---
"The exact details of the representation are unspecified and subject
to change, ..." So, that's the way it *ought* to work.  

Regarding FileObject.toString, the javadoc does a good job
of following these guidelines: [snip]

This make it quite clear that developers should not rely
on the output of FileObject.toString.  So, why the need to
deprecate it? I can see temporarily deprecating it to make
it easier to find and examine all uses, but I don't see a
need to leave it deprecated.
---%<---

My new suggestion is in line with your message, I think. The new impl
continues to return an unspecified string representation, and forces
those who misread the Javadoc to fix their code (in earlier releases
as well as 4.0).

Re. temporary vs. permanent deprecation: we can temporarily deprecate
something to find all *our* uses perhaps, but this is useless for most
third-party module developers, who normally download and use an FCS
release to test their module against. The third-party developers are
anyway the ones we need to pay the most attention to when considering
deprecations; it is easy enough for us Sun employees to scan
nb_all/f4j_all sources for poor usages, and know what to do with them.
Others may not have heard about the problems surrounding classpath
changes.


Somewhat off-topic but:

---%<---
[snip] final will break source compatability
and enforces violation of the Item 9 which encourages 
all classes to provide a useful toString.  Subclasses of
FileObject will not be able to do this.
---%<---

I think Effective Java here just means that Object.toString() should
always be overridden, which is already true here - not that every
subclass needs to override it from its parent. In fact most
filesystems extend AbstractFileSystem and do not provide their own
FileObject subclasses (AFS provides a private impl for them). The use
of FileSystem.systemName effectively captures the most useful
information from the filesystem anyway.
Comment 14 David Strupl 2002-12-10 08:03:36 UTC
I agree with returning whatever string you propose. I use it quite
often for debugging purposes and your proposed solution is even better
since it tells me the filesystem (which I am usually interested in
anyway).
Comment 15 Jesse Glick 2004-01-27 23:13:03 UTC
David probably you would have the most interest in this issue at this
point?
Comment 16 Jesse Glick 2004-04-02 22:09:46 UTC
This has been done. Now shows e.g.

  AbstractFileObject@123456[file:/tmp/foo.txt]
Comment 17 Marian Mirilovic 2005-12-20 15:53:07 UTC
This issue was solved long time ago. Because nobody has reopened it neither
added comments, we are verifying/closing it now. 
If you are still able to reproduce the problem, please reopen. Thanks in advance.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo