Skip to content

Commit

Permalink
Add Analyzer to disallow ^= true
Browse files Browse the repository at this point in the history
  • Loading branch information
YoshiRulz committed Jul 8, 2024
1 parent c99d221 commit 2a5d4b9
Show file tree
Hide file tree
Showing 38 changed files with 257 additions and 242 deletions.
2 changes: 2 additions & 0 deletions .global.editorconfig.ini
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ dotnet_diagnostic.BHI1101.severity = error
dotnet_diagnostic.BHI1102.severity = error
# Don't call typeof(T).ToString(), use nameof operator or typeof(T).FullName
dotnet_diagnostic.BHI1103.severity = error
# Don't use ^= (XOR-assign) for inverting the value of booleans
dotnet_diagnostic.BHI1104.severity = error
# Brackets of collection expression should be separated with spaces
dotnet_diagnostic.BHI1110.severity = warning
# Expression-bodied member should be flowed to next line correctly
Expand Down
79 changes: 79 additions & 0 deletions ExternalProjects/BizHawk.Analyzer/UseSimplerBoolFlipAnalyzer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
namespace BizHawk.Analyzers;

using System.Collections.Immutable;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

/// <remarks>shoutouts to SimpleFlips</remarks>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class UseSimplerBoolFlipAnalyzer : DiagnosticAnalyzer
{
private const string ERR_MSG_SIMPLE = "Use e.g. `b = !b;` instead of `b ^= true;`";

private const string ERR_MSG_COMPLEX = $"{ERR_MSG_SIMPLE} (you may want to store part of the expression in a local variable to avoid repeated side-effects or computation)";

private static readonly DiagnosticDescriptor DiagUseSimplerBoolFlip = new(
id: "BHI1104",
title: "Don't use ^= (XOR-assign) for inverting the value of booleans",
messageFormat: "{0}",
category: "Usage",
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(DiagUseSimplerBoolFlip);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
ISymbol? boolSym = null;
context.RegisterOperationAction(
oac =>
{
static bool IsZeroWorkLocalOrFieldRef(IOperation trunk)
{
while (trunk.Kind is OperationKind.FieldReference)
{
if (trunk.ChildOperations.Count is 0) return true; // in the unit test, the node(s) for the implicit `this.` are missing for some reason
trunk = trunk.ChildOperations.First();
}
return trunk.Kind is OperationKind.InstanceReference or OperationKind.LocalReference;
}
var operation = (ICompoundAssignmentOperation) oac.Operation;
if (operation.OperatorKind is not BinaryOperatorKind.ExclusiveOr) return;
boolSym ??= oac.Compilation.GetTypeByMetadataName("System.Boolean")!;
if (!boolSym.Matches(operation.Type)) return;
if (operation.Value.Kind is not OperationKind.Literal) return;
var lhsOp = operation.Target;
bool lhsIsSimpleExpr;
switch (lhsOp.Kind)
{
case OperationKind.PropertyReference:
lhsIsSimpleExpr = false;
break;
case OperationKind.FieldReference:
lhsIsSimpleExpr = IsZeroWorkLocalOrFieldRef(lhsOp);
break;
case OperationKind.LocalReference:
lhsIsSimpleExpr = true;
break;
case OperationKind.ArrayElementReference:
lhsIsSimpleExpr = false;
break;
default:
oac.ReportDiagnostic(Diagnostic.Create(DiagUseSimplerBoolFlip, operation.Syntax.GetLocation(), $"Left-hand side of XOR-assign was of an unexpected kind: {lhsOp.GetType().FullName}"));
return;
}
oac.ReportDiagnostic(Diagnostic.Create(
DiagUseSimplerBoolFlip,
operation.Syntax.GetLocation(),
lhsIsSimpleExpr ? DiagnosticSeverity.Error : DiagnosticSeverity.Warning,
additionalLocations: null,
properties: null,
lhsIsSimpleExpr ? ERR_MSG_SIMPLE : ERR_MSG_COMPLEX));
},
OperationKind.CompoundAssignment);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
namespace BizHawk.Tests.Analyzers;

using System.Threading.Tasks;

using Microsoft.VisualStudio.TestTools.UnitTesting;

using Verify = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier<
BizHawk.Analyzers.UseSimplerBoolFlipAnalyzer,
Microsoft.CodeAnalysis.Testing.DefaultVerifier>;

[TestClass]
public sealed class UseSimplerBoolFlipAnalyzerTests
{
[TestMethod]
public Task CheckMisuseOfXORAssignment()
=> Verify.VerifyAnalyzerAsync("""
public static class Cases {
private static int _z = default;
private static bool _a = default;
private static void CasesMethod() {
_z ^= 0xA5A5;
_a ^= DummyMethod();
{|BHI1104:_a ^= true|};
var b = false;
{|BHI1104:b ^= true|};
bool c = default;
{|BHI1104:c ^= false|}; // this is effectively a no-op so there's no reason it would be used in the first place, but it was easier to flag both
{|BHI1104:AnotherClass.GetInstance().Prop ^= true|}; // care needs to be taken with non-trivial expressions like this; a different message will be given
}
private static bool DummyMethod()
=> default;
}
public sealed class AnotherClass {
public static AnotherClass GetInstance()
=> new();
public bool Prop { get; set; }
private AnotherClass() {}
}
""");
}
Binary file modified References/BizHawk.Analyzer.dll
Binary file not shown.
5 changes: 1 addition & 4 deletions src/BizHawk.Client.Common/Controller.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,7 @@ public void Overrides(OverrideAdapter controller)
_axes[button] = controller.AxisValue(button);
}

foreach (var button in controller.InversedButtons)
{
_buttons[button] ^= true;
}
foreach (var button in controller.InversedButtons) _buttons[button] = !_buttons[button];
}

public void BindMulti(string button, string controlString)
Expand Down
4 changes: 1 addition & 3 deletions src/BizHawk.Client.Common/RecentFiles.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ public bool Remove(string newFile)
}

public void ToggleAutoLoad()
{
AutoLoad ^= true;
}
=> AutoLoad = !AutoLoad;
}
}
2 changes: 1 addition & 1 deletion src/BizHawk.Client.Common/SaveSlotManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public void ClearRedoList()

public void ToggleRedo(IMovie movie, int slot)
{
if (slot is >= 1 and <= 10 && movie is not ITasMovie) _redo[slot - 1] ^= true;
if (slot is >= 1 and <= 10 && movie is not ITasMovie) _redo[slot - 1] = !_redo[slot - 1];
}

public bool IsRedo(IMovie movie, int slot)
Expand Down
2 changes: 1 addition & 1 deletion src/BizHawk.Client.Common/tools/Cheat.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public void Toggle(bool handleChange = true)
{
if (!IsSeparator)
{
_enabled ^= true;
_enabled = !_enabled;
if (handleChange)
{
Changes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ public IEnumerable<ToolStripItem> GenerateContextMenuItems()
ShortcutKeyDisplayString = RotateHotkeyStr
};

rotate.Click += (o, ev) => { HorizontalOrientation ^= true; };
rotate.Click += (_, _) => HorizontalOrientation = !HorizontalOrientation;

yield return rotate;
}
Expand Down Expand Up @@ -1372,10 +1372,7 @@ protected override void OnKeyDown(KeyEventArgs e)
}
else if (e.IsCtrlShift(Keys.F))
{
if (Rotatable)
{
HorizontalOrientation ^= true;
}
if (Rotatable) HorizontalOrientation = !HorizontalOrientation;
}
// Scroll
else if (e.IsPressed(Keys.PageUp))
Expand Down
2 changes: 1 addition & 1 deletion src/BizHawk.Client.EmuHawk/Extensions/ToolExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ public static class ToolExtensions
Text = recent.Frozen ? "&Unfreeze" : "&Freeze",
Image = recent.Frozen ? Properties.Resources.Unfreeze : Properties.Resources.Freeze
};
freezeItem.Click += (o, ev) => recent.Frozen ^= true;
freezeItem.Click += (_, _) => recent.Frozen = !recent.Frozen;
items.Add(freezeItem);

if (!noAutoload)
Expand Down
60 changes: 21 additions & 39 deletions src/BizHawk.Client.EmuHawk/MainForm.Events.cs
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,10 @@ private void QuickLoadstateMenuItem_Click(object sender, EventArgs e)
private void LoadNamedStateMenuItem_Click(object sender, EventArgs e) => LoadStateAs();

private void AutoloadLastSlotMenuItem_Click(object sender, EventArgs e)
{
Config.AutoLoadLastSaveSlot ^= true;
}
=> Config.AutoLoadLastSaveSlot = !Config.AutoLoadLastSaveSlot;

private void AutosaveLastSlotMenuItem_Click(object sender, EventArgs e)
{
Config.AutoSaveLastSaveSlot ^= true;
}
=> Config.AutoSaveLastSaveSlot = !Config.AutoSaveLastSaveSlot;

private void SelectSlotMenuItems_Click(object sender, EventArgs e)
{
Expand Down Expand Up @@ -510,14 +507,10 @@ private void StopMovieWithoutSavingMenuItem_Click(object sender, EventArgs e)
}

private void AutomaticMovieBackupMenuItem_Click(object sender, EventArgs e)
{
Config.Movies.EnableBackupMovies ^= true;
}
=> Config.Movies.EnableBackupMovies = !Config.Movies.EnableBackupMovies;

private void FullMovieLoadstatesMenuItem_Click(object sender, EventArgs e)
{
Config.Movies.VBAStyleMovieLoadState ^= true;
}
=> Config.Movies.VBAStyleMovieLoadState = !Config.Movies.VBAStyleMovieLoadState;

private void MovieEndFinishMenuItem_Click(object sender, EventArgs e)
{
Expand Down Expand Up @@ -601,9 +594,7 @@ private void ScreenshotClientClipboardMenuItem_Click(object sender, EventArgs e)
}

private void ScreenshotCaptureOSDMenuItem_Click(object sender, EventArgs e)
{
Config.ScreenshotCaptureOsd ^= true;
}
=> Config.ScreenshotCaptureOsd = !Config.ScreenshotCaptureOsd;

private void ExitMenuItem_Click(object sender, EventArgs e)
{
Expand Down Expand Up @@ -728,25 +719,19 @@ private void DisplayInputMenuItem_Click(object sender, EventArgs e)
}

private void DisplayRerecordsMenuItem_Click(object sender, EventArgs e)
{
Config.DisplayRerecordCount ^= true;
}
=> Config.DisplayRerecordCount = !Config.DisplayRerecordCount;

private void DisplaySubtitlesMenuItem_Click(object sender, EventArgs e)
{
Config.DisplaySubtitles ^= true;
}
=> Config.DisplaySubtitles = !Config.DisplaySubtitles;

private void DisplayStatusBarMenuItem_Click(object sender, EventArgs e)
{
Config.DispChromeStatusBarWindowed ^= true;
Config.DispChromeStatusBarWindowed = !Config.DispChromeStatusBarWindowed;
SetStatusBar();
}

private void DisplayMessagesMenuItem_Click(object sender, EventArgs e)
{
Config.DisplayMessages ^= true;
}
=> Config.DisplayMessages = !Config.DisplayMessages;

private void DisplayLogWindowMenuItem_Click(object sender, EventArgs e)
{
Expand Down Expand Up @@ -959,7 +944,7 @@ private void ProfilesMenuItem_Click(object sender, EventArgs e)

private void ClockThrottleMenuItem_Click(object sender, EventArgs e)
{
Config.ClockThrottle ^= true;
Config.ClockThrottle = !Config.ClockThrottle;
if (Config.ClockThrottle)
{
var old = Config.SoundThrottle;
Expand All @@ -982,7 +967,7 @@ private void ClockThrottleMenuItem_Click(object sender, EventArgs e)

private void AudioThrottleMenuItem_Click(object sender, EventArgs e)
{
Config.SoundThrottle ^= true;
Config.SoundThrottle = !Config.SoundThrottle;
RewireSound();
if (Config.SoundThrottle)
{
Expand All @@ -1000,7 +985,7 @@ private void AudioThrottleMenuItem_Click(object sender, EventArgs e)

private void VsyncThrottleMenuItem_Click(object sender, EventArgs e)
{
Config.VSyncThrottle ^= true;
Config.VSyncThrottle = !Config.VSyncThrottle;
_presentationPanel.Resized = true;
if (Config.VSyncThrottle)
{
Expand All @@ -1024,7 +1009,7 @@ private void VsyncThrottleMenuItem_Click(object sender, EventArgs e)

private void VsyncEnabledMenuItem_Click(object sender, EventArgs e)
{
Config.VSync ^= true;
Config.VSync = !Config.VSync;
if (!Config.VSyncThrottle) // when vsync throttle is on, vsync is forced to on, so no change to make here
{
_presentationPanel.Resized = true;
Expand All @@ -1038,14 +1023,12 @@ private void UnthrottledMenuItem_Click(object sender, EventArgs e)

private void ToggleUnthrottled()
{
Config.Unthrottled ^= true;
Config.Unthrottled = !Config.Unthrottled;
ThrottleMessage();
}

private void MinimizeSkippingMenuItem_Click(object sender, EventArgs e)
{
Config.AutoMinimizeSkipping ^= true;
}
=> Config.AutoMinimizeSkipping = !Config.AutoMinimizeSkipping;

private void NeverSkipMenuItem_Click(object sender, EventArgs e) { Config.FrameSkip = 0; FrameSkipMessage(); }
private void Frameskip1MenuItem_Click(object sender, EventArgs e) { Config.FrameSkip = 1; FrameSkipMessage(); }
Expand Down Expand Up @@ -1781,9 +1764,7 @@ private void N64ControllerSettingsMenuItem_Click(object sender, EventArgs e)
}

private void N64CircularAnalogRangeMenuItem_Click(object sender, EventArgs e)
{
Config.N64UseCircularAnalogConstraint ^= true;
}
=> Config.N64UseCircularAnalogConstraint = !Config.N64UseCircularAnalogConstraint;

private static void Mupen64PlusSetMupenStyleLag(bool newValue, ISettingsAdapter settable)
{
Expand Down Expand Up @@ -2450,7 +2431,7 @@ private void ClearSramContextMenuItem_Click(object sender, EventArgs e)

private void ShowMenuContextMenuItem_Click(object sender, EventArgs e)
{
MainMenuStrip.Visible ^= true;
MainMenuStrip.Visible = !MainMenuStrip.Visible;
FrameBufferResized();
}

Expand Down Expand Up @@ -2558,8 +2539,9 @@ private void LinkConnectStatusBarButton_Click(object sender, EventArgs e)
// toggle Link status (only outside of a movie session)
if (!MovieSession.Movie.IsPlaying())
{
Emulator.AsLinkable().LinkConnected ^= true;
Console.WriteLine("Cable connect status to {0}", Emulator.AsLinkable().LinkConnected);
var core = Emulator.AsLinkable();
core.LinkConnected = !core.LinkConnected;
Console.WriteLine($"Cable connect status to {core.LinkConnected}");
}
}

Expand Down
11 changes: 7 additions & 4 deletions src/BizHawk.Client.EmuHawk/MainForm.Hotkey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ void SelectAndLoadFromSlot(int slot)
RebootCore();
break;
case "Toggle Skip Lag Frame":
Config.SkipLagFrame ^= true;
Config.SkipLagFrame = !Config.SkipLagFrame;
AddOnScreenMessage($"Skip Lag Frames toggled {(Config.SkipLagFrame ? "On" : "Off")}");
break;
case "Toggle Key Priority":
Expand Down Expand Up @@ -382,15 +382,18 @@ void SelectAndLoadFromSlot(int slot)
break;
case "Toggle Follow Cursor":
if (!Tools.IsLoaded<TAStudio>()) return false;
Tools.TAStudio.TasPlaybackBox.FollowCursor ^= true;
var playbackBox = Tools.TAStudio.TasPlaybackBox;
playbackBox.FollowCursor = !playbackBox.FollowCursor;
break;
case "Toggle Auto-Restore":
if (!Tools.IsLoaded<TAStudio>()) return false;
Tools.TAStudio.TasPlaybackBox.AutoRestore ^= true;
var playbackBox1 = Tools.TAStudio.TasPlaybackBox;
playbackBox1.AutoRestore = !playbackBox1.AutoRestore;
break;
case "Toggle Turbo Seek":
if (!Tools.IsLoaded<TAStudio>()) return false;
Tools.TAStudio.TasPlaybackBox.TurboSeek ^= true;
var playbackBox2 = Tools.TAStudio.TasPlaybackBox;
playbackBox2.TurboSeek = !playbackBox2.TurboSeek;
break;
case "Undo":
if (!Tools.IsLoaded<TAStudio>()) return false;
Expand Down
2 changes: 1 addition & 1 deletion src/BizHawk.Client.EmuHawk/MainForm.Movie.cs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ private void ToggleReadOnly()
{
if (MovieSession.Movie.IsActive())
{
MovieSession.ReadOnly ^= true;
MovieSession.ReadOnly = !MovieSession.ReadOnly;
AddOnScreenMessage(MovieSession.ReadOnly ? "Movie read-only mode" : "Movie read+write mode");
}
else
Expand Down
Loading

0 comments on commit 2a5d4b9

Please sign in to comment.