Skip to content

Convert quick fixes to bulk actions #1649

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

Merged
merged 13 commits into from
May 7, 2020
Merged

Conversation

citizenmatt
Copy link
Member

The following Quick Fixes have been converted into bulk (scoped) actions, that work across method, class, file, folder, project and solution:

  • Convert to CompareTag
  • Convert to generic overload (GameObject.GetComponent, GameObject.AddComponent, ScriptableObject.CreateInstance)
  • Combine setting parent with object creation ("Use Instantiate with parent argument")
  • Convert coalescing operator to conditional
  • Remove redundant attribute

The remove redundant attribute quick fix is now a "redundant code" quick fix and has additional features/implications:

  • The quick fix is executed as part of removing redundant code in Code Cleanup
  • Adds "remove redundant code in XXX" bulk fixes, which remove ALL redundant code, such as unnecessary this., dead code, etc.
  • The bulk quick fix will remove all redundant Unity attributes in scope, not just the attribute type under the caret, so it will remove any incorrectly applied [SerializeField], [InitializeOnLoad], [HideInInspector], etc.

@citizenmatt citizenmatt added this to the Rider 2020.2 milestone May 6, 2020
@citizenmatt citizenmatt requested a review from krasnotsvetov May 6, 2020 09:51
@citizenmatt citizenmatt self-assigned this May 6, 2020
@citizenmatt
Copy link
Member Author

citizenmatt commented May 6, 2020

Another implication: if a non-serialisable base class has a [SerializeField] attribute, we currently consider that as redundant - we don't check for derived classes that might be serialised. The remove redundant attribute bulk fix will remove this attribute, even though it is valid.

This needs to become a show-stopper: RIDER-18729

(See #1650 for tracking in the 2020.2 milestone. I want to make sure this isn't lost)

@@ -19,9 +18,15 @@ protected UnityElementProblemAnalyzer([NotNull] UnityApi unityApi)

protected sealed override void Run(T element, ElementProblemAnalyzerData data, IHighlightingConsumer consumer)
{
// TODO: Check other process kinds. Should we run for everything?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krasnotsvetov can you verify what kinds we should be ignoring here? We obviously want to work for visible document and SWEA. These changes require OTHER to quickly find highlightings to fix, but what about the other values?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, that we should ignore INCREMENTAL_SOLUTION_ANALYSIS, but ignoring GLOBAL_WARNINGS is ok. Only performance critical inspections (and burst in future) will use global stage

@citizenmatt citizenmatt marked this pull request as ready for review May 6, 2020 10:09
Copy link
Collaborator

@krasnotsvetov krasnotsvetov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@citizenmatt citizenmatt merged commit 019629e into net202 May 7, 2020
@citizenmatt citizenmatt deleted the feature/scoped-actions branch May 7, 2020 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants