stefanmaron / businesscentral.lintercop Goto Github PK
View Code? Open in Web Editor NEWCommunity driven code linter for AL (MS Dynamics 365 Business Central)
Home Page: https://stefanmaron.com
License: MIT License
Community driven code linter for AL (MS Dynamics 365 Business Central)
Home Page: https://stefanmaron.com
License: MIT License
I'm getting the error Cannot read property 'trim' of undefined
under Runtime Status
in vscode.
If I run the DownloadFile.ps1
directly I get some more errors ...
Invoke-RestMethod : The request was aborted: Could not create SSL/TLS secure channel.
At C:\Users\User\.vscode\extensions\stefanmaron.businesscentral-lintercop-0.1.0\DownloadFile.ps1:7 char:18
+ ... stRelease = Invoke-RestMethod -Uri "https://api.github.com/repos/Stef ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : InvalidOperation: (System.Net.HttpWebRequest:HttpWebRequest) [Invoke-RestMethod], WebExc
eption
+ FullyQualifiedErrorId : WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeRestMethodCommand
The machine I'm running on is a Windows Server 2016, the actual problem is that admin needs to set the below to allow Powershell to use TLS1.2.
Windows Registry Editor Version 5.00
[HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\v4.0.30319]
"SchUseStrongCrypto"=dword:00000001
[HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\.NETFramework\v4.0.30319]
"SchUseStrongCrypto"=dword:00000001
[HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\v2.0.50727]
"SystemDefaultTlsVersions"=dword:00000001
[HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\.NETFramework\v2.0.50727]
"SystemDefaultTlsVersions"=dword:00000001
Now the PS1 script runs fine standalone, but I'm still getting the message.
It seems there's another issue ...
C:\Users\User\.vscode\extensions\stefanmaron.businesscentral-lintercop-0.1.0\DownloadFile.ps1 : Cannot dot-source
this command because it was defined in a different language mode. To invoke this command without importing its
contents, omit the '.' operator.
At line:1 char:1
+ . C:\Users\User\.vscode\extensions\stefanmaron.businesscentral-lint ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : InvalidOperation: (:) [DownloadFile.ps1], NotSupportedException
+ FullyQualifiedErrorId : DotSourceNotSupported,DownloadFile.ps1
Omitting the .
operator makes it work.
Perhaps it would be best to do the download using Javacript within VSCode?
Right now, the linter is creating warnings. Is there some way to make them errors or even better, can we define which issues should be errors and which should be warnings?
Hi @StefanMaron,
It appears that with the new release LC0005 can't decide if it wants to have currXMLPort
or CurrXmlPort
. Could you resolve this? Perhaps accept both while we are at it? ๐ (simply because CurrXmlPort
looks better, more consistent ๐)
The LC0015 rule warns if you are missing the execute (X) permissions on pages, codeunits etc., which is great!
But for tables it seems as if only the TableData is being checked.
I think that the execute permissions on Table should be checked as well, so that both TableData and Table must be defined in at least one PermissionSet for each object in the extension.
@StefanMaron, as per the short discussion on you blog I am posting this question here: how about cyclomatic complexity? Or in other words: what became of this feature that was part of the AL Lint VS Code extension? Or is it in there and I do not understand how to get it triggered (btw: not expecting it as LinterCop is a cop)?
BTW: see #47, where I note thta the read.me is a bit sparse on telling what LinterCop is actually doing.
Maybe it is me, but the read.me does not tell what LinterCop checks. Could this info be provided?
I am getting the following error after upgrading to the latest version of the LinterCop in one of my projects:
[{
"resource": "/c:/[redacted]/app.json",
"owner": "_generated_diagnostic_collection_name_#0",
"code": "AD0001",
"severity": 4,
"message": "Analyzer 'BusinessCentral.LinterCop.Design.Rule0017WriteToFlowField' threw an exception of type 'System.InvalidCastException' with message 'System.InvalidCastException: Unable to cast object of type 'Microsoft.Dynamics.Nav.CodeAnalysis.BoundTextIndexAccess' to type 'Microsoft.Dynamics.Nav.CodeAnalysis.IFieldAccess'.
at BusinessCentral.LinterCop.Design.Rule0017WriteToFlowField.CheckForWriteToFlowField(OperationAnalysisContext context)
at Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__DisplayClass53_1.<ExecuteOperationAction>b__1() in D:\a\1\s\source\Prod\Microsoft.Dynamics.Nav.CodeAnalysis\DiagnosticAnalyzer\AnalyzerExecutor.cs:line 764
at Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock(DiagnosticAnalyzer analyzer, Action analyze, Nullable`1 info) in D:\a\1\s\source\Prod\Microsoft.Dynamics.Nav.CodeAnalysis\DiagnosticAnalyzer\AnalyzerExecutor.cs:line 1100'",
"source": "AL",
"startLineNumber": 1,
"startColumn": 1,
"endLineNumber": 1,
"endColumn": 1
}]
Please let me know if you need any files uploaded to troubleshoot this, I can't really make out what the issue is here.
EDIT: It should be noted that this happens with one specific project after upgrading. I cannot reproduce this in any of my other projects.
This warning will push developers to make sure all public extension API is documented or to make sure that it is not public. It's even more important for AppSource apps as all public procedures are locked for future modifications if they are public so will act as an additional notification to set internal access modifier if it's not intended to be an app public API.
Wrong casing is not recognized in statements like DATABASE::"Permission set":
On every table the properties LookupPageIdand
DrillDownPageId
should be defined if a list page is defined for that table.
Here's another thing I found when using LC0015:
In the warning for LC0015 it prints warning LC0015: The application object Record My Table is not covered by any permission set in the extension.
when the TableData for the table is not included in any PermissionSet.
I would suggest to change the warning to warning LC0015: The application object TableData "My Table" is not covered by any permission set in the extension.
So two changes:
Now I'm done adding issues for today. ;-)
Keep up the good work and have a great weekend!
I have the following in my codeunit and receive a wrong warning for permission declaration.
warning LC0003: Do not use an Object ID for properties or variables declaration. Use "tabledata "PRI-KA Entry" = R,
tabledata "Incoming Document" = RIM,
tabledata "Incoming Document Attachment" = RIMD" instead.
report 50000 "PRI-KA Process Entries"
{
Caption = 'Process ntries';
UsageCategory = None;
ProcessingOnly = true;
Permissions = tabledata "PRI-KA Entry" = R,
tabledata "Incoming Document" = RIM,
tabledata "Incoming Document Attachment" = RIMD;
dataset
{
dataitem(Entry; "PRI-KA Entry")
{
trigger OnAfterGetRecord()
begin
end;
}
}
}
There is are reasons for an Commit but then you have to write it down.
(a spin off from #48)
I believe that all the rules deserves their own .md file that documents them.
Our goal with this is actually to don't have any use of your CodeCop, because all code is perfect already ๐
This documentation would help developers to understand why a warning appears and what to do to resolve them (and avoid them in the future).
As inspiration for headers in these files (not all for every rule, but one or more per rule):
Microsoft is using all of above in different rule descriptions on MS Docs.
Hi @StefanMaron,
This one is a bit tricky. Microsoft is being inconsistent in the AL Language for the casing of XmlPort
.
For variable declarations they expect XmlPort
:
var
MyXmlPort: Xmlport "Export Contact";
For when you use IntelliSense to type a statement it suggests Xmlport
:
But then when you hover over the following, it is expected to be written as XmlPort
:
XmlPort.Export(Xmlport::"Export Contact", MyOutStream);
Xmlport.Export(XmlPort::"Export Contact", MyOutStream);
But note that for the first part of these statements, the LC0005 code analyzer doesn't even mind. ;)
And when you would write it like this (which looks the best) and hover over it, then it 'suggests' Xmlport
once more:
XmlPort.Export(XmlPort::"Export Contact", MyOutStream);
How do you think the LC0005 code should handle this? Should it allow both, or should we approach it differently?
Hi @StefanMaron,
I think LC0004 should not apply for tables of TableType = Temporary;
, because there is no point in drilling down or doing a lookup.
Hi @StefanMaron,
It seems like rule LC0003 gives a false positive on array variables. Here is an example:
var
CustCalendarChange: array[2] of Record "Customized Calendar Change";
Really nice to find all those places where a NotBlank
has been missed! ๐
But two things:
NotBlank = false
should suppress this warning. In that way we could allow blank PK for the setup tables. Right now I get a lot of warnings about NotBlank is missing from all setup tables, and I want to avoid pragmas as long as possible and rather have explicit declarations...In AL the compiler does not complain if the casing in variable usage differs from definition.
Hi,
I have just installed the extension i VS Code, and enabled the code analyzer.
I'm getting the following exception, this goes both for existing projects, and a brand new one created with AL:Go
Analyzer 'BusinessCentral.LinterCop.Design.Rule0012DoNotUseObjectIdInSystemFunctions' threw an exception of type 'System.IndexOutOfRangeException' with message 'System.IndexOutOfRangeException: Index was outside the bounds of the array. at System.Collections.Immutable.ImmutableArray1.get_Item(Int32 index) at BusinessCentral.LinterCop.Design.Rule0012DoNotUseObjectIdInSystemFunctions.CheckForObjectIdsEventSubscribers(SymbolAnalysisContext context) at Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__DisplayClass49_2.<ExecuteSymbolActionsCore>b__0() in D:\a\1\s\source\Prod\Microsoft.Dynamics.Nav.CodeAnalysis\DiagnosticAnalyzer\AnalyzerExecutor.cs:line 656 at Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock(DiagnosticAnalyzer analyzer, Action analyze, Nullable1 info) in D:\a\1\s\source\Prod\Microsoft.Dynamics.Nav.CodeAnalysis\DiagnosticAnalyzer\AnalyzerExecutor.cs:line 1078'
The auto deployment of the Linter Cop DLL is really neat!
But when deploying fixes it might be a bit to easy to deploy a fix with regression issues or that a new rule that is not yet 100% perfect creates zillions of new warnings that will just confuse users until they understand the rule completely.
So I suggest that a new boolean setting is added to the VSCode extension, Install Preview releases
, or similar.
If this setting is active, the dll should be downloaded from another location - to where you deploy preview versions of the dll.
In that way you can get things tested out by a group of volunteers and get feedback before deploying to all users.
I would always have the preview release installed... ๐
Thoughts?
Page Definition
page 50100 "Some Table List"
{
SourceTable = "Some Table";
PageType = List;
ApplicationArea = All;
UsageCategory = Lists;
AccessByPermission = tabledata Item = R;
}
Warning shown:
Do not use an Object ID for properties or variables declaration. Use tabledata Item = R instead.
Tested on release version: v0.23.0 (https://github.com/StefanMaron/BusinessCentral.LinterCop/releases/tag/v0.23.0)
Originally posted by KristofKlein September 25, 2021
A table object can be set - per definition - to be temporary only.
It behaves as a normal table object while it gets designed. But not while runtime! We found out, that it allows you to define a primary key with auto increment. But since the auto increment is a feature that the SQL server executes it is actually not working for temp tables (like in CAL).
So It would be nice to get a hint, that it is not possible to use an auto. inc. ...or guide into the direction to use a number sequence instead?
Reading the message
Do not use an Object ID for properties or variables declaration. Use NVT Electronic Document Setup instead.
I would expect an object ID being used, that needs replacements.
However, the object name is used, but the casing is not respected (it should have been written as 'NVT Electronic Document Setup' instead). For me this seems like an invalid candidate for the LC0003 rule and fits more the LC0005 rule (which we've excluded)
A function should not be called Validate, SetRange or an Field should not be named "Count".
Hi @StefanMaron,
Just ran into the following false positive for LC0012. I only expect this rule to give a false positive if a constant integer is used.
It also throws a warning if a procedure-call is used as an argument.
Example:
local procedure RunSetupPage()
begin
Page.RunModal(GetSetupPage());
end;
local procedure GetSetupPage(): Integer
begin
exit(Page::"General Ledger Setup");
end;
Hi @StefanMaron,
It seems like rule LC0003 gives a false positive on the AccessByPermission
for the following sample:
action("Reservation Entries")
{
AccessByPermission = TableData Item = R;
ApplicationArea = Reservation;
Caption = 'Reservation Entries';
Image = ReservationLedger;
ToolTip = 'View all reservations that are made for the item, either manually or automatically.';
trigger OnAction()
begin
Rec.ShowReservationEntries(true);
end;
}
Could you have a look?
Thank you for the code analyzer and extension! :)
Hi @StefanMaron,
Running into issues with rule LC0012 which reports a problem for any event subscriber.
Example:
[EventSubscriber(ObjectType::Codeunit, Codeunit::"Guided Experience", 'OnRegisterAssistedSetup', '', false, false)]
local procedure CodeunitGuidedExperience_OnRegisterAssistedSetup()
begin
/// ... some statement that invokes a procedure
end;
Wrong Parameter detected. Select the correct object with "ObjectType::Codeunit;Codeunit::"Guided Experience"::" instead.
The Access
property is only available for Enum
and Interface
objects for runtime 7.0 and greater.
Accordingly we should have rule only apply for enums and interfaces when the runtime is set to 7.0 or greater, otherwise the rule should skip these object types.
Following code results in a LC0016 warning for the label control, although the text is set through the CaptionClass property.
group(Instructions)
{
ShowCaption = false;
Visible = not _OpenClicked;
Editable = false;
label(InstructionText)
{
ApplicationArea = All;
CaptionClass = GetInstructionText();
Style = Strong;
StyleExpr = true;
}
There is no good reason to use an ID as SourceTable or in variables, parameters instead of the Names.
Hi @StefanMaron,
I reported this one earlier to you via a DM, but I think you might have missed it in the final release.
The following variable/parameter declaration gives a false positive:
var vXMLNode: DotNet XmlNode
The alias is XmlNode
which should be allowed, and is preferred.
It would also be fine for me to skip DotNet variables completely though.
Hi @StefanMaron,
Rule LC0013 raises the following error for us:
Analyzer 'BusinessCentral.LinterCop.Design.Rule0013CheckForNotBlankOnSingleFieldPrimaryKeys' threw an exception of type 'System.InvalidCastException' with message 'System.InvalidCastException: Unable to cast object of type 'Microsoft.Dynamics.Nav.CodeAnalysis.Symbols.SourceTableExtensionTypeSymbol' to type 'Microsoft.Dynamics.Nav.CodeAnalysis.ITableTypeSymbol'.
at BusinessCentral.LinterCop.Design.Rule0013CheckForNotBlankOnSingleFieldPrimaryKeys.CheckForSingleFieldPrimaryKeysNotBlank(SymbolAnalysisContext context)
at Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics.AnalyzerExecutor.<>c__DisplayClass49_2.<ExecuteSymbolActionsCore>b__0() in D:\a\1\s\source\Prod\Microsoft.Dynamics.Nav.CodeAnalysis\DiagnosticAnalyzer\AnalyzerExecutor.cs:line 656
at Microsoft.Dynamics.Nav.CodeAnalysis.Diagnostics.AnalyzerExecutor.ExecuteAndCatchIfThrows_NoLock(DiagnosticAnalyzer analyzer, Action analyze, Nullable`1 info) in D:\a\1\s\source\Prod\Microsoft.Dynamics.Nav.CodeAnalysis\DiagnosticAnalyzer\AnalyzerExecutor.cs:line 1100'
This can be reproduced with an app with a table extension on the Item table, e.g.:
tableextension 50100 ItemExt extends Item
{
fields
{
field(50100; MyField; Code[20])
{
Caption = 'MyField';
DataClassification = CustomerContent;
}
}
}
Hi @StefanMaron,
Ran into one more scenario where both LC0003 and LC0005 give a false positive. Which is for a DotNet
variable declaration.
For example:
MyXMLDocument: DotNet XmlDocument;
(p.s., I am aware that there is an XmlDocument
object type in AL, but this is just an example I ran into ๐)
LC0015 warns on tables with TableType = Temporary
.
A user does not need permissions to write or read data in temporary tables, so I believe that LC0015 should skip the warning if there are no PermissionSet with TableData for that table.
The PTE0004 rule works in that way already.
When using AL Language extension from nextmajor (v9.0.582246) BC.LinterCop starts to flood me with false positives.
These are the ones I seen so far:
RecRef.Open(MyRecord.MyFieldNo)
It's not urgent yet, but it might be something to start looking into. :)
If the Code is not correct indented, it should be at least an warning.
The LC0005 rule is really great! It helped me to find a lot of casing issues.
But the last update introduced a few false positives.
I at least found three of them (maybe there are more):
Action
datatypeInterface
datatypePage
datatypeAll those datatypes should begin with a capital letter, but LC0005 now thinks it should be all lowercase.
The following repro gets three warnings:
procedure MyProcedure(MyAction: Action)
var
MyInterface: Interface "Price Asset";
CustomerCard: Page "Customer Card";
begin
end;
The common thing with those keywords is that they all are both data types and another keyword. Don't know if that's the cause, though...
action
is used in pages.interface
and page
is object types.I do not know if this is even possible, so just close it as "won't fix" if it isn't. ๐
I would really like to start using LinterCop on all our AppSource apps. But when activating LinterCop on a project with very old code, we get flooded with warnings.
Activating LinterCop with a "big bang" where we just sit down and go through thousands of warnings will not happen anytime soon, it will just take too much of the time we do not currently have.
Activating one rule at a time will still cause a lot of warnings to get through, depending on the rule of course. This will also limit the gain of using LinterCop...
This creates a big hurdle for us to start adopting LinterCop, and I assume that most ISVs will experience the same hurdle.
So here's my feature request for solving above:
Add a new setting to LinterCop.json, IgnoreFiles
or similar, that is an array of glob patterns.
When running LinterCop on an .al file this setting is checked to see if the warnings in that file should be ignored.
In this way I can structure, and re-structure, my .al files in a way that makes LinterCop to be active in the files that I actively work on and in the files that has already been fixed to comply with all the rules.
Thoughts? Is it even technically possible?
Thanks,
Johannes
Hi @StefanMaron,
Here are some examples for which LC0005 could raise warnings for keyword-casing in a future release ๐:
AcTions
{
ArEa(ProceSsing)
{
AcTion(MyAction)
{
APPLICATIONAREA = All;
ImaGe = Approval;
TrigGer OnActiOn()
begin
PaGe.RunModal(Page::"Customer Card");
Page.RunMOdal(Page::"Item Card");
end;
}
See #92.
But I agree, an explicit NotBlank = false should also satisfy the rule.
It would be good to change the text to:
"The NotBlank property should be set explicitly for tables with a single-field primary key."
to avoid confusion.
The newly introduced rule gives a warning of a missing caption on system parts, such as Links, while I'm pretty sure those are already added by the application. Manually-added normal page parts such as factboxes are properly detected by the rule.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.