Repository navigation
Report undeclared names and code that is not strict Modelica 3.7 - #27
Merged
Merged
Conversation
Two opt-in rules for libraries that must load in tools other than Dymola. MLQT.Reference.Undeclared (UndeclaredNameAnalyzer, Error when on) reports each name used where nothing of that name is declared: variables in equations, algorithms, bindings, conditions and subscripts; functions called; types, bases and imports; members after a dot, enumeration literals included; and the targets of modifications, checked against a predefined type's attributes where the type is one. It follows the roadmap's three-state rule: a miss is reported only when every scope the lookup passed through is readable with all its bases loaded, no unresolved import could bind the name, and no unloaded library could supply it (a plain variable, or a name uses(...) rules out). Anything reached through replaceable, expandable or Complex is never judged. Measured: nothing reported over MSL 4.1.0, ModelicaTest and Buildings, 99% of MSL's 200k name uses judged, 4.6 s over Buildings. MLQT.Conformance.Grammar (GrammarConformance, Warning when on) reports each place a class parses only because modelica.g4 accepts more than MLS 3.7 Appendix A: a leading class annotation, a short-class prefix other than input/output, ':=' in a modification, a der statement other than der(x) := e, a comprehension after several arguments, escapes MLS does not define, white space in a quoted identifier, and time or a predefined type's name used as an identifier. settings-reference.md lists them, and the places where the grammar is narrower than 3.7 instead. firstTick and Subtask join ModelicaLanguage's built-in names (MLS §16). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MLQT.Reference.Undeclared reported every use of a tool's own functions and flags, which is right for a library that must load anywhere and noise for one written for a particular tool. AcceptedToolBuiltIns names the tools whose built-ins count as declared; empty, the default, accepts none, and the finding then says which tool the name belongs to. ToolBuiltIns (ModelicaParser.Helpers) holds the names: Dymola's built-in functions, its Advanced and Hidden flag packages and the Dymola_AST_ function family, matched by prefix as there is no complete list; and OpenModelica's OpenModelica package. A tool's name is asked only after lookup has found nothing, so a library's own class of that name still wins, and a scope that is not complete still leaves it unjudged. The Static analysis section shows a checkbox per tool while the rule is on; the MCP style settings take the list, where only null leaves it alone; and ChecksDifferFrom counts a change to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The accepted-tools checkboxes and the package.order "match Dymola" switch each change one rule's findings, and sat below the whole list, where nothing said which rule they belonged to. Each now follows its own rule's row, chosen by the RuleIds constant rather than a literal id. Also pins that Dymola's flags read and assigned in an algorithm count as declared when Dymola is accepted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With Dymola accepted, Advanced.Translation.X was still reported wherever a class had import Modelica.Utilities.Strings.*: MSL has a Strings.Advanced package, so lookup bound Advanced to it and failed on Translation, and a tool's names were only offered where the first segment resolved to nothing. Dymola reads its flags whatever an import brings in, so a name that is the tool's whole is now the tool's however far lookup got: declared where the tool is accepted, and said to be the tool's where it is not. A name the libraries resolve completely still means their class. A listed Dymola name now also matches as the first segment, so the literals of its plot enumerations (LinePattern.Solid, MarkerStyle.None) are Dymola's too. Measured over a large user repository of 21,249 classes with Dymola accepted: no finding about a Dymola name is left. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MudBlazor widens a divider's grab area with a ::before reaching 12px past every side of the 4px gap. That lay over the first pane's scrollbar - the tree's vertical one beside the tabs, the code viewer's horizontal one above the findings - so pressing either started a drag and only the wheel scrolled. Our dividers now grow their grab area into the second pane only, and sit above it with z-index: 1. Without that, a positioned second pane (Code Review's findings pane) paints over the ::before and the divider could be grabbed by its gap alone. ResizablePanesJourney checks both halves at every one of our dividers, and fails against MudBlazor's own grab area. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TypeResolver's predefined list was the language's plus Complex, so every class built on it - ComplexInput = input Complex, SI.ComplexVoltage = Complex(...) - had no members as far as the resolver knew: u.re resolved to nothing, and the units on re and im were never seen. MLS 3.7 §4.9 does not list Complex; it is the operator record of Complex.mo, a library beside MSL, and now resolves as any class does. Without Complex.mo loaded it is unresolved, as any library not loaded is. UndeclaredNameAnalyzer no longer works round it, so members through Complex are judged like any record's. CreateSimpleTypeLookup keeps ordering a Complex declaration among the variables, as it always has, so Format All rearranges nothing. Measured: the resolution corpus resolves all 125,158 component types (181 more than before, the Complex ones) with no redeclaration changing a unit; MSL and Buildings with every rule on report the same findings, rule for rule, at the same speed. Removing the workaround without this fix gave 129 false findings on MSL's ComplexBlocks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two opt-in checks for libraries that have to load in tools other than Dymola, a setting that lets a library accept the built-ins of the tools it is written for, and a resolver fix those checks exposed.
MLQT.Reference.Undeclared(Error when on)Reports a name used where nothing of that name is declared:
s.missing), enumeration literals includedSub s(gian = 1), andReal x(fixd = true)against the predefined type's attributes)It follows the roadmap's three-state rule. A miss is reported only when every scope searched is readable, with all bases loaded, and no unresolved import could bind the name. It also needs no unloaded library to be able to supply the name: either a plain variable, or a name the library's
uses(...)rules out. Members reached throughreplaceableor anexpandableconnector are never judged.Accepting a tool's built-ins. While the rule is on, the Static analysis section shows a Dymola and an OpenModelica checkbox, saved as
AcceptedToolBuiltIns. The default, neither, reports every use and names the tool. Dymola covers its built-in functions, theAdvanced/Hiddenflags,Dymola_AST_*and its plot enumerations; OpenModelica covers itsOpenModelicapackage. A tool's name counts even where an import binds part of it:import Modelica.Utilities.Strings.*brings in MSL'sStrings.Advanced, and Dymola still readsAdvanced.Translation.Xas a flag. The CLI and the MCP style settings take the list too.MLQT.Conformance.Grammar(Warning when on)modelica.g4accepts what Dymola accepts beyond the MLS 3.7 concrete syntax (Appendix A, keywords §2.3.3). This rule reports each place a class relies on one of the eight widenings:input/output:=in a modificationderstatement that is notder(x) := etimeor a predefined type's name used as an identifiersettings-reference.md§Strict Modelica 3.7 syntax lists them, and the places where the grammar is narrower than 3.7 (those are already syntax errors).Complexis resolved, not predefinedTypeResolver.IsPredefinedcountedComplexas predefined, so every class built on it had no members:u.reresolved to nothing and the units onre/imwere unseen. MLS 3.7 §4.9 does not list it; it is the operator record ofComplex.mo. It now resolves like any class. Declaration ordering still treats aComplexdeclaration as a variable, so Format All moves nothing.Also on this branch
ResizablePanesJourneycovers it).Measured
ModelicaTest, Buildings: neither new rule reports anything. 99% of MSL's ~200k name uses get a definite verdict, so the zero is a real result rather than silence.Complexones); no package redeclaration changes a unit.Complexchange: MSL and Buildings report the same findings, rule for rule, at the same speed.Test plan
CI=true dotnet build MLQT.slnx -c Release: no warningsbuild/run-all-tests.ps1 -CoreOnly: 9,546 tests passbuild/check-coverage.ps1: gate passesMLQT_RESOLUTION_CORPUS)🤖 Generated with Claude Code