[219] Make override QuickFix doesn't change access modifier


BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Given this code:
public class Base
{
protected virtual void SetUp()
{
}
}


public class CreateCommandTest : Base
{

public void
SetUp()
{
}
}

If I apply the 'Make 'SetUp' override' QuickFix, the resulting code is:

...


public override void
SetUp()
{
}

...

Which is incorrect, as SetUp() is protected.

Regards,
Pablo
-


BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (MingW32)

iD8DBQFECM/bvooSiBfQCSoRAvyLAKD3Wa84WgsAP9cHecv+1nkwBuEI5wCg6NId
eSOB2ONvEWD8h4++UXKY2go=
=IM9j
-


END PGP SIGNATURE-----

8 comments
Comment actions Permalink

Do you think the quickfix should change visibility modfier in this case?
I'm not sure.

Valentin Kipiatkov
Chief Scientist, Vice President of Product Development
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

---BEGIN PGP SIGNED MESSAGE---
Hash: SHA1

Given this code:
public class Base
{
protected virtual void SetUp()
{
}
}

public class CreateCommandTest : Base
{

public void
SetUp()
{
}
}
If I apply the 'Make 'SetUp' override' QuickFix, the resulting code
is:

...


public override void
SetUp()
{
}
...

Which is incorrect, as SetUp() is protected.

Regards,
Pablo
---BEGIN PGP SIGNATURE---

>> Version: GnuPG v1.2.1 (MingW32)
>>
>> iD8DBQFECM/bvooSiBfQCSoRAvyLAKD3Wa84WgsAP9cHecv+1nkwBuEI5wCg6NId
>> eSOB2ONvEWD8h4++UXKY2go=
>> =IM9j

---END PGP SIGNATURE---



0
Comment actions Permalink

"Valentin Kipiatkov (JetBrains)" <valentin@jetbrains.com> wrote in message
news:3fdb29a6aed4a8c817439857d342@news.intellij.net...

Do you think the quickfix should change visibility modfier in this case?
I'm not sure.


Using the same visibility as the overridden method does seem more logical.
Though it would be clearer if in this case, the quickfix was named "make
protected override".
Another alternative would be not to display this quickfix if the
visibilities are different.

I'd see any of those as an improvement over generating code that doesn't
compile :)
(a little preference for the first option, since the quickfix would be just
as easy to use, generate correct code and the intent would be clearer)


Regards,

                Lionel


0
Comment actions Permalink

Hello Valentin,

VK> Do you think the quickfix should change visibility modfier in this
VK> case? I'm not sure.

As for me, I think it should.

Sincerely,
Ilya Ryzhenkov


0
Comment actions Permalink

IMO decision about changing visibility should be approved by the user. Current
approach effectively gives the user such possibility by showing next error
after applying the first fix and suggesting the second one.

Valentin Kipiatkov
Chief Scientist, Vice President of Product Development
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

Hello Valentin,

VK>> Do you think the quickfix should change visibility modfier in this
VK>> case? I'm not sure.
VK>>

As for me, I think it should.

Sincerely,
Ilya Ryzhenkov



0
Comment actions Permalink

I'd see any of those as an improvement over generating code that
doesn't
compile :)


But you see immediate feedback about mismatching visibility by highlighting
'public' keyword after applying the quickfix. And may make a decision at
this moment.

Valentin Kipiatkov
Chief Scientist, Vice President of Product Development
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

"Valentin Kipiatkov (JetBrains)" <valentin@jetbrains.com> wrote in
message news:3fdb29a6aed4a8c817439857d342@news.intellij.net...

>> Do you think the quickfix should change visibility modfier in this
>> case? I'm not sure.
>>

Using the same visibility as the overridden method does seem more
logical.
Though it would be clearer if in this case, the quickfix was named
"make
protected override".
Another alternative would be not to display this quickfix if the
visibilities are different.
I'd see any of those as an improvement over generating code that
doesn't
compile :)
(a little preference for the first option, since the quickfix would be
just
as easy to use, generate correct code and the intent would be clearer)
Regards,

Lionel



0
Comment actions Permalink

I'd see any of those as an improvement over generating code that
doesn't
compile :)


But you see immediate feedback about mismatching visibility by highlighting
'public' keyword after applying the quickfix. And may make a decision at
this moment.

Valentin Kipiatkov
Chief Scientist, Vice President of Product Development
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"

"Valentin Kipiatkov (JetBrains)" <valentin@jetbrains.com> wrote in
message news:3fdb29a6aed4a8c817439857d342@news.intellij.net...

>> Do you think the quickfix should change visibility modfier in this
>> case? I'm not sure.
>>

Using the same visibility as the overridden method does seem more
logical.
Though it would be clearer if in this case, the quickfix was named
"make
protected override".
Another alternative would be not to display this quickfix if the
visibilities are different.
I'd see any of those as an improvement over generating code that
doesn't
compile :)
(a little preference for the first option, since the quickfix would be
just
as easy to use, generate correct code and the intent would be clearer)
Regards,

Lionel



0
Comment actions Permalink

BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Valentin Kipiatkov (JetBrains) wrote:

IMO decision about changing visibility should be approved by the user.
Current approach effectively gives the user such possibility by showing
next error after applying the first fix and suggesting the second one.


I understand your point of view, but the user gets the same ability to
change visibility in the first time when presented with the two possible
quickfixes, namely the 'override' and the 'make new' (which will leave
the visibility unchanged), which are the only two actions that are
correct in the given context.

Both quick fixes should generate compiling code. After all, if you apply
a quickfix and get an error isn't it transformed in a quickerror instead?

Regards,
Pablo
-


BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (MingW32)

iD8DBQFEGtXPvooSiBfQCSoRAmj7AKCw56tTp4Mlhug4AEUaD3pA1WCOxACgnV8v
LdHzsbzAbYMlpL/RQbKihFg=
=HgiY
-


END PGP SIGNATURE-----

0
Comment actions Permalink

BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Valentin Kipiatkov (JetBrains) wrote:

IMO decision about changing visibility should be approved by the user.
Current approach effectively gives the user such possibility by showing
next error after applying the first fix and suggesting the second one.


I understand your point of view, but the user gets the same ability to
change visibility in the first time when presented with the two possible
quickfixes, namely the 'override' and the 'make new' (which will leave
the visibility unchanged), which are the only two actions that are
correct in the given context.

Both quick fixes should generate compiling code. After all, if you apply
a quickfix and get an error isn't it transformed in a quickerror instead?

Regards,
Pablo
-


BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (MingW32)

iD8DBQFEGtXPvooSiBfQCSoRAmj7AKCw56tTp4Mlhug4AEUaD3pA1WCOxACgnV8v
LdHzsbzAbYMlpL/RQbKihFg=
=HgiY
-


END PGP SIGNATURE-----

0

Please sign in to leave a comment.