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

New PTE Rule: Table Extension fields should be in app id ranges (expand PTE0002) #7547

Closed
3 tasks done
mjmatthiesen opened this issue Oct 17, 2023 · 2 comments
Closed
3 tasks done
Labels
accepted PerTenantExtensionCop This is a specific static-code-analysis group (PTE) ships-in-future-update The issue is fixed in our 'master' branch and will ship in the next PREVIEW release static-code-analysis

Comments

@mjmatthiesen
Copy link

Please include the following with each issue:

1. Describe the bug
Create a new app. Make your range 71100-71200. Create a table extension. Add a field. Make the id anything within the 50k-99k range.

2. To Reproduce
Steps to reproduce the behavior:

tableextension 71100 "Sales Header Ext." extends "Sales Header"
{
    fields
    {
        field(51100; "New Field"; Code[50]) { }
    }
}

3. Expected behavior
This should produce a warning as you are outside of the specific apps range. This is bad practise.

4. Actual behavior
No warning, builds fine, no feedback to dev that they have messed up.

5. Versions:

  • AL Language: v12.1.883011
  • Visual Studio Code:Version: 1.83.1 (system setup)
  • Business Central: irrelevant
  • List of Visual Studio Code extensions that you have installed: All of them

Final Checklist

Please remember to do the following:

  • Search the issue repository to ensure you are reporting a new issue

  • Reproduce the issue after disabling all extensions except the AL Language extension

  • Simplify your code around the issue to better isolate the problem

@EmilDamsbo
Copy link
Member

Agreed, this should behave in line with AppSource Cop rule AS0013. However, it would also be a breaking change for existing AL code to make it an error. I think a new rule is required which by default is a warning.

@EmilDamsbo EmilDamsbo added static-code-analysis PerTenantExtensionCop This is a specific static-code-analysis group (PTE) accepted labels Oct 18, 2023
EmilDamsbo added a commit to microsoft/BCApps that referenced this issue Jan 15, 2024
#### Summary 
This AL compiler issue microsoft/AL#7547 is
being addressed with the introduction of a new rule validated enum
ordinal values falls within the allocated id ranges. This should fix
this violation

#### Work Item(s)
Fixes
[AB#487991](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/487991)
@JesperSchulz JesperSchulz added the ships-in-future-update The issue is fixed in our 'master' branch and will ship in the next PREVIEW release label Jan 25, 2024
@JesperSchulz
Copy link

The fix for this issue has been checked in to the master branch. It will be available in the bcinsider.azurecr.io/bcsandbox-master Docker image starting from platform build number 15401 and VS Code Extension Version .

If you don’t have access to these images you need to become part of the Ready2Go program: aka.ms/readytogo

For more details on code branches and docker images please read:
https://blogs.msdn.microsoft.com/nav/2018/05/03/al-developer-previews-multiple-releases-and-github/
https://freddysblog.com/2020/06/25/working-with-artifacts/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted PerTenantExtensionCop This is a specific static-code-analysis group (PTE) ships-in-future-update The issue is fixed in our 'master' branch and will ship in the next PREVIEW release static-code-analysis
Projects
None yet
Development

No branches or pull requests

3 participants