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

Feature/atr 8 dev no bql field #544

Merged
merged 60 commits into from
Aug 29, 2024
Merged

Conversation

SENya1990
Copy link
Contributor

@SENya1990 SENya1990 commented Aug 23, 2024

Added PX1065 diagnostic to check DAC field properties for missing BQL fields + refactored common logic out of legacy BQL fields and constants analyzers.

Overview:

PX1065 diagnostic:

  • Added PX1065 diagnostic to check DAC field properties for missing BQL fields
  • Added a code fix for PX1065 to generate a BQL field if the reported property is declared inside the analyzed DAC.
  • Added unit tests for PX1065 and its code fix for DACs and DAC extensions
  • Added tests for PX1065 with DACs and DAC extension that derives or extend DACs from the external DLL
  • Wrote documentation for PX1065

Test Infrastructure:

  • Added external solution to the repo to imitate external dependency. Tests now are referencing the external dependency DLL. The dynamically generated test workspace also references external dependency DLL.
  • Fixed bug in CodeFixVerifier that caused code fix verification to exit prematurely if the first diagnostic didn't have any code actions registered
  • Added capability to override the number of attempts to apply code actions in CodeFixVerifier

Semantic Model

  • Moved common logic for mapping between DAC property type and BQL field type into a separate
    PropertyTypeToBqlFieldTypeMapping helper
  • Moved common logic for BQL field generation into utils and reworked legacy BQL field and constant code fixes.
  • Extended DacFieldAttributeInfo with IsAcumaticaAttribute flag + refactoring
  • Extended IOverridableItem interface with CombineWithBaseInfo to add to existing semantic infos initial support for combination with their base members. The combination should calculate effective properties of the semantic info object based on its declared properties and properties of its base info object. Implemented initial support for this combination.
  • Extended DAC semantic model and renamed part of its properties to be more clear on terminology
  • Extended DAC property and DAC field infos with IsAcumaticaAttribute flag
  • Fixed big issue in the DAC field infos collection. The combination with the base DAC field info wasn't done previously.

Other Code Analysis Changes

  • Reworked MissingTypeListAttributeAnalyzer to support aggregator attributes and use precalculated data about them. PX1002 check became more precise.
  • Fixed mass code fixes for diagnostics:
    • PX1012
    • PX1062
    • PX1072

Utilities Changes:

  • Refactoring - renamed string type name constants and symbols with misleading names
  • Added read only hash set wrapper for hashset
  • Small fixes in helpers

Other:

  • Updated existing diagnostic docs - replaced IBqlTable with PXBqlTable

…et and works faster on average than the immutable hashset
…m semantic model for aggregated field attributes
…operties and added HasAcumaticaAttributes property
…ype with Contains methods and intergrated changes.

Details:
- extended mappping helper between property type and BQL field type with Contains methods that check if type is present in mapping tables
- integrated changes into legacy BQL field and constant analyzers
- removed mapping table from the BQL field analyzer since it is moved to the mapping helper in utilities
…same as the Acumatica types and to reflect that they are interfaces
@@ -402,4 +402,7 @@
<data name="PX1022Graph" xml:space="preserve">
<value>NonPublicGraph</value>
</data>
<data name="PX1065" xml:space="preserve">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@EAndrosova please review the diagnostic short name

@@ -702,4 +702,10 @@ public virtual int? SomeDacField{ get; set; }</value>
<data name="PX1022GraphExtensionFix" xml:space="preserve">
<value>Make the graph extension public</value>
</data>
<data name="PX1065TitleFormat" xml:space="preserve">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@EAndrosova please review the diagnostic and code fix messages

Copy link
Collaborator

Choose a reason for hiding this comment

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

What about adding a test for a property that does not have any PXEventSubscriberAttribute-inherited attribute and checking that the diagnostic is not reported on such property since it is not "DAC field"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great idea! I'll add it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, after I added new tests, I decided to also add a tests with base DAC in external dll, so I've integrated one into the repo and tests. And with it I discovered a huge bug in the way this version of Roslyn does collection of data from the external symbols. ISymbol.GetTypeMembers method returns type itself instead of nested types.

So, it was a great idea to add new tests.

The verifier previously used only the first diagnostic in the set to get available code actions. If that diagnostic couldn't provide any code action to apply, then the verifier stopped. This led to a situation when the first diagnostic in the code source didn't provide an action, and the verifier stops even if the second diagnostic can provide valid code actions
This solution should be used to test Acuminator data collection and analysis from the external dll
@SENya1990 SENya1990 linked an issue Aug 27, 2024 that may be closed by this pull request
@SENya1990 SENya1990 merged commit a0f44a9 into dev Aug 29, 2024
@SENya1990 SENya1990 deleted the feature/ATR-8-dev-no-BQL-field branch August 29, 2024 19:20
This pull request was closed.
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.

Create a check for DACs to valide fields
3 participants