Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize moves variables out of #pragma warning blocks #239

Closed
bjorndavis opened this issue Mar 30, 2016 · 6 comments
Closed

Reorganize moves variables out of #pragma warning blocks #239

bjorndavis opened this issue Mar 30, 2016 · 6 comments

Comments

@bjorndavis
Copy link

Environment

  • Visual Studio version: 2013
  • CodeMaid version: 0.9.1
  • Code language: C#

Description

When using the reorganize command, CodeMaid moves variables out of #pragma warning blocks.

Steps to recreate

Before reorganize:

public class Test
    {
        private string test2;
#pragma warning disable 0649
        private string test1;
#pragma warning restore 0649
    }

After reorganize:

public class Test
    {
        private string test1;
        private string test2;

#pragma warning disable 0649
#pragma warning restore 0649
    }

Current behavior

In certain specific cases it's desirable to disable compiler warnings for a section of code (e.g. disabling 0649 - "Field is never assigned to" when using the Selenium PageFactory model., as the variables are assigned using attributes). However, CodeMaid will move variables out of these sections, which is undesirable.

Expected behavior

Re-organize command should keep items within #pragma warning sections if they were there to begin with.

@codecadwallader
Copy link
Owner

Thanks for reporting the issue, I have reproduced it.

When we detect other preprocessor conditionals (e.g. #if) we disable reorganization so it does not incorrectly manipulate the file. We will now pick up #pragma statements as well during that detection and disable reorganization of files where that statement is present.

We have a new CI channel that will automatically generate builds based on the latest code snapshot. If you would like to confirm the fix you can download it from here: http://vsixgallery.com/extension/4c82e17d-927e-42d2-8460-b473ac7df316/ Please note this is a CI channel so it is bleeding edge code and may exhibit other issues that the current released version does not. If you encounter issues you can simply uninstall this version and re-install the latest stable version.

@bjorndavis
Copy link
Author

Disabling CodeMaid Reorganize entirely seems to be a fairly drastic step - is it possible to simply treat #pragma warning blocks like #regions (or at least enable/disable this in the settings)? Unfortunately I'll have to stick to an older version as I have a fair amount of code that has to use the #pragmas, and I'll be unable to re-organize otherwise.

@codecadwallader
Copy link
Owner

Understood, but following the concept of "first do no harm" I think it's better to not perform a reorganization operation at all than to potentially perform it incorrectly. A long term solution that includes comprehension of #pragma statements, what impacts they make on the code and how they should be treated falls into the same camp as #if statements. That will be dependent on an upcoming Roslyn rewrite to have a richer code model, but that's a very large effort that won't be accomplished anytime soon to be honest.

An alternative workaround is that you can fork the code and override the recent change, or stick to older builds as you have mentioned. I understand that's not what you want to hear, but adding full model awareness for #pragma statements unfortunately isn't something I can quickly address.

@bjorndavis
Copy link
Author

That's fair enough, thanks for taking the time to reply.

@AliveDevil
Copy link

I've never had a problem with #pragma-Blocks and would like to have this function back in CodeMaid, because Unity Development can get very annoying.
If everything in reorganize is in a #pragma-Block, than nothing goes wrong.
I got around the given issue with

#region Pragma Warning Disable 0649
#pragma warning disable 0649
// fields
#pragma warning restore 0649
#endregion

That used to be an alternative. So: please add an option to enable reorganize again. The developer should know if he wants to take the risk or not.

@codecadwallader
Copy link
Owner

I understand your perspective. Will you please create a new issue (and reference this one) for the feature request to have an option that is off by default and allows overriding the safety checks? As this is an open source project please feel free to tackle implementation if you would like to speed it along.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants