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.

Sign up for GitHub

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

Jump to bottom

[core] Explicitly name all language versions #4120

Closed
Tracked by #3898
oowekyala opened this issue Sep 11, 2022 · 6 comments · Fixed by #4387
Closed
Tracked by #3898

[core] Explicitly name all language versions #4120

oowekyala opened this issue Sep 11, 2022 · 6 comments · Fixed by #4387
Labels
an:enhancement An improvement on existing features / rules
Milestone
7.0.0

Comments

@oowekyala
Copy link
Member

oowekyala commented Sep 11, 2022 •
edited by adangel
Loading

Is your feature request related to a problem? Please describe.
Some languages have a single language version with the empty string as a name. This is not a problem today but #4059 aims to make --use-version more useful by allowing to explicitly state a version to use for all languages. The goal of this is to have a failsafe in case the version is changed or removed, and to document versions better. This is not possible if versions have empty names.

Describe the solution you'd like
Give an explicit name for all language versions.

Describe alternatives you've considered
#4048 was an alternative to this but doesn't help with the --use-version use case described above, indeed making it worse.

Additional context
See full thread in #4059 (comment)

PMD Language and version mappings

Language Id=Terse Name (Name) Extensions Rules Version(s) in PMD 6 (*default) Proposed Version(s) in PMD 7
apex (Apex) cls, trigger ✔️ 54* 52, 53, 54, 55, 56, 57*
ecmascript (Ecmascript) js ✔️ 3* 3, 5, 6 (alias: ES6, ES2015), 7 (ES2016), 8 (ES2017), 9* (ES2018)
html (HTML) html, htm, xhtml, xht, shtml ✔️ ""* 4, 5*
java (Java) java ✔️ 1.3, 1.4, 1.5, 5, 1.6, 6, 1.7, 7, 1.8, 8, 9, 1.9, 10, 1.10, 11, 12, 13, 14, 15, 16, 17, 18, 18-preview, 19*, 19-preview same
jsp (Java Server Pages) jsp, jspx, jspf, tag ✔️ ""* 2, *3
kotlin (Kotlin) kt, ktm ✔️ n/a - only PMD 7 1.6, 1.7*
modelica (Modelica) mo ✔️ ""* 3.4, 3.5*
plsql (PLSQL) sql, trg, prc, fnc, pld, pls, plh, plb, pck, pks, pkh, pkb, typ, tyb, tps, tpb ✔️ ""* 11g, 12c Release 1 (alias: 12.1), 12c Release 2 (alias 12.2), 18c, 19c, 21c*
text (Plain text) plain-text-file-goo-extension ❌ default* ❓
pom (Maven POM) pom ✔️ ""* 4.0.0*
scala (Scala) scala ❌ 2.13*, 2.12, 2.11, 2.10 same
swift (Swift) swift (:x:) :heavy_check_mark: n/a (only CPD) 4.2, 5.0, 5.1, 5.2, 5.3, 5.4, 5.5, 5.6, 5.7*
vf (Salesforce VisualForce) page, component ✔️ ""* 52, 53, 54, 55, 56, 57*
vm (VM) (=Velocity Template Language) vm ✔️ ""* 2.0, 2.1, 2.2, 2.3*
wsdl (WSDL) wsdl ❌ ""* 1.1, 2.0*
xml (XML) xml ✔️ ""* 1.0, 1.1*
xsl (XSL) xsl, xslt ✔️ ""* 1.0, 2.0, 3.0*

PMD Languages that only support CPD

all except "go" are deprecated on PMD 6, as they don't support PMD
They will be/have been deleted in PMD 7

Language Id=Terse Name (Name) Extensions
cpp (C++) h, c, cpp, cxx, cc, C
cs (C#) cs
fortran (Fortran) for, f, f66, f77, f90
go (Golang) go
groovy (Groovy) groovy
matlab (Matlab) m
objectivec (Objective-C) h, m
php (PHP: Hypertext Preprocessor) php, class
python (Python) py
ruby (Ruby) rb, cgi, class
swift (Swift) swift

CPD Languages

Language Id=Terse Name (Name) Extensions
any (Any Language) n/a
apex (Apex) .cls
cpp (C++) .h, .hpp, .hxx, .c, .cpp, .cxx, .cc, .C
cs (C#) .cs
dart (Dart) .dart
ecmascript (JavaScript) .js
fortran (Fortran) .for, .f, .f66, .f77, .f90
gherkin (Gherkin) .feature
go (Go) .go
groovy (Groovy) .groovy
html (HTML) .html
jsp (JSP) .jsp, .jspx, .jspf, .tag
java (Java) .java
kotlin (Kotlin) .kt
lua (Lua) .lua
matlab (Matlab) .m
modelica (Modelica) .mo
objectivec (Objective-C) .h, .m
php (PHP) .php, .class
plsql (PL/SQL) .sql, .trg, .prc, .fnc, .pld, .pls, .plh, .plb, .pck, .pks, .pkh, .pkb, .typ, .tyb, .tps, .tpb
perl (Perl) .pm, .pl, .t
python (Python) .py
ruby (Ruby) .rb, .cgi, .class
scala (Scala) .scala
swift (Swift) .swift
vf (VisualForce) .page, .component
xml (XML) .xml
@oowekyala oowekyala added the an:enhancement An improvement on existing features / rules label Sep 11, 2022
@oowekyala oowekyala added this to the 7.0.0 milestone Sep 11, 2022
@oowekyala oowekyala mentioned this issue Sep 11, 2022
Closed
@jsotuyod
Copy link
Member

jsotuyod commented Sep 12, 2022 •
edited
Loading

@oowekyala I edited the issue to include a list of languages and versions to set when we pick this up.

I took the freedom of setting values for those we know / can for sure. We can discuss others on this thread and add them there as a summary as we reach agreements.

Open questions:

  • HTML should work for HTML5 and 4.01… it should also work for XHTML 1.1, do we handle that as a different version somehow? or should it be a completely different language?
  • As for VTL, and given the nature of it (they version the engine, and changes to VTL are usually limited to reserved var names / predefined variables, with plenty of configuration flags to revert back to previous behavior), it may be good to just list "2.x" as version and not tie to a particular release.
  • Visualforce: Visualforce and Apex versions go hand in hand. They are released at the same time and share the same number, so normally this would currently be "54", but is there a way for us to enforce both languages stay in sync?

@jsotuyod jsotuyod mentioned this issue Oct 17, 2022
Merged
16 tasks
@oowekyala oowekyala mentioned this issue Nov 22, 2022
Merged
8 tasks
@adangel adangel added this to To do in PMD 7 Jan 9, 2023
@adangel adangel modified the milestones: 7.0.0, 7.x Jan 23, 2023
@adangel adangel removed this from To do in PMD 7 Jan 23, 2023
@jsotuyod
Copy link
Member

@adangel I'd argue this actually is required for PMD 7.0.0 release… otherwise, the --use-version arg for these languages don't comply with the documentation and would change in a breaking way in future releases.

pmd/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java

Lines 32 to 41 in 510f41a

SUPPORTED_LANGUAGES_PMD = "Valid values: apex-54, ecmascript-ES6, html-," + System.lineSeparator()
+ " java-1.10, java-1.3, java-1.4, java-1.5, java-1." + System.lineSeparator()
+ " 6, java-1.7, java-1.8, java-1.9, java-10," + System.lineSeparator()
+ " java-11, java-12, java-13, java-14, java-15," + System.lineSeparator()
+ " java-16, java-17, java-18, java-18-preview," + System.lineSeparator()
+ " java-19, java-19-preview, java-5, java-6, java-7," + System.lineSeparator()
+ " java-8, java-9, jsp-, kotlin-1.6, kotlin-1." + System.lineSeparator()
+ " 6-rfc+0.1, modelica-, plsql-, pom-, scala-2.10," + System.lineSeparator()
+ " scala-2.11, scala-2.12, scala-2.13, swift-, vf-," + System.lineSeparator()
+ " vm-, wsdl-, xml-, xsl-";

We may not be exhaustive in PMD 7 (although the cost is probably marginal), but at least 1 version per language would be required for proper compliance.

@adangel
Copy link
Member

adangel commented Jan 24, 2023 •
edited
Loading

Yes, that makes sense. The language-version-combination html- looks weird.... I'll add it to the list. Maybe it can be/is part of #4060.

@adangel adangel modified the milestones: 7.x, 7.0.0 Jan 24, 2023
@adangel adangel mentioned this issue Jan 24, 2023
Closed
55 tasks
@adangel
Copy link
Member

adangel commented Feb 3, 2023 •
edited
Loading

Let me try to summarize the discussion:

  • we don't want to have "empty" versions anymore, that is "". This results in invalid language-version strings when
    selecting a version at the CLI via --use-version.
    • no "versionless" modules anymore
  • we don't ever talk about "defaults"
    • one of the versions will be marked as default - it will be used, if the user doesn't select a specific version.
      But it won't be named "default".
    • the default version might change in new versions of PMD. It is usually a minor version update (not breaking).
      That's what we do in Java.
    • the "empty" version is not equal to "default"
  • naming a version "latest" is also not productive - it has similar consequences as providing a version named "default".

Approaching a solution:

  • What do the version represent?

    • IMHO, they represent the languages' version. If we in PMD change the implementation or make breaking change in
      the AST (like in Java), these versions don't change.
    • They should be represent "well-known" versions for the language, whatever this means.
    • They say, which versions of the language is supported by PMD. E.g. we can mostly parse the version.
      That's one usage of the version in pmd-java - since newer java version often change the grammar, which requires
      a parser change.

  • What are the versions used for?

    • in pmd-java for two things:
      • configuring the parser to only allow language syntax for the given version.
      • enabling/disabling rules, that only apply to specific language versions (minimumLanguageVersion, maximumLanguageVersion).
        This includes Java API changes (e.g. a rule suggesting a fix that is only possible since a specific Java version)
    • the only other language with multiple versions is pmd-scala. But there we don't have any rules. However,
      the parser is configured with the specific version, so there is a difference in theory.
    • no other language module than pmd-java uses minimumLanguageVersion / maximumLanguageVersion for rules
      • However, some Apex rules could use this
    • My assumption: Basically, when a user selects a specific language version when running PMD, he wants to make sure,
      that he don't sees violations, that are not relevant (because a problem was only relevant for older language
      versions).

Suggestion:

  • For each language, we should list always all versions, that PMD can parse without immediate problems (the remaining
    problems can just be considered as usual bugs - like we have for Java or PL/SQL).
  • That means e.g. for Apex, we list many versions. Starting now, we might just use the last 5 version, as I think the
    older versions are not really relevant.
  • Since only pmd-java uses minimumLanguageVersion / maximumLanguageVersion, selecting a specific version of other
    languages has virtually no effect right now - PMD does the same thing.
  • We could (that's probably for PMD 8) consider, to not differentiate in pmd-java's parser between different version
    and always parse with the latest language features enabled. We assume anyway, that we only see compilable code, so
    we don't need to check this again. The language version would then really only be used to select applicable rules.

Suggested tasks:

  • Research official versions for our supported languages. If there is no version, use the year of the
    latest specification. Document the location of the specifications.
  • Add the versions. Always select the latest version as "default".
  • Make the version name be required - empty version names are not allowed anymore
  • Make sure for each PMD language, that the names are reused for the CPD language (e.g. Ecmascript / JavaScript)
  • Make sure for each PMD language, that the same extensions are used for CPD language
  • Change extension for pom to xml (fixes [pom] Maven POM Rules don't check files named "pom.xml" #1540)
    • we probably have anyway some extension clashes, at least for CPD (cpp vs. objectivec)

Wdyt?

@adangel
Copy link
Member

adangel commented Feb 3, 2023 •
edited
Loading

I've done the research now and collected the specs etc.

apex:

Proposed versions: 57, 56, 55, 54, 53, 52

ecmascript:

Proposed versions: 3, 5, 6 (alias: ES6, ES2015), 7 (ES2016), 8 (ES2017), 9 (ES2018)

html:

Proposed versions: 5, 4

java:

jsp:

Proposed versions: 2, 3, 4?

kotlin:

modelica:

plsql:

pom:

Proposed versions: 4.0.0

scala:

swift:

Proposed versions: 4.2, 5.0, 5.1, 5.2, 5.3, 5.4, 5.5, 5.6, 5.7

Salesforce VisualForce:

  • Visualforce Developer Guide
    • "The Visualforce framework includes a tag-based markup language similar to HTML"
  • Same versions as Apex (57.0 (Spring '23))

Proposed versions: 57, 56, 55, 54, 53, 52

vm:

Proposed versions: 2.0, 2.1, 2.2, 2.3

wsdl:

Proposed versions: 1.1, 2.0

xml:

Proposed versions: 1.0, 1.1

xsl:

Proposed versions: 1.0, 2.0, 3.0

@adangel adangel mentioned this issue Feb 3, 2023
Merged
adangel added a commit to adangel/pmd that referenced this issue Feb 9, 2023
@adangel
[core] Require explicit language versions (pmd#4120)
58ce62e
adangel added a commit to adangel/pmd that referenced this issue Feb 9, 2023
@adangel
Add explicit language versions (pmd#4120)
7a7da1c
@adangel adangel mentioned this issue Feb 9, 2023
Merged
9 tasks
@adangel adangel linked a pull request Feb 9, 2023 that will close this issue
[core] Explicitly name all language versions #4387
Merged
9 tasks
adangel added a commit to adangel/pmd that referenced this issue Feb 9, 2023
@adangel
[doc] Update release notes (pmd#4120) [skip ci]
5aeee8b
adangel added a commit to adangel/pmd that referenced this issue Feb 10, 2023
@adangel
[core] Require explicit language versions (pmd#4120)
79d2380
adangel added a commit to adangel/pmd that referenced this issue Feb 10, 2023
@adangel
Add explicit language versions (pmd#4120)
4252597
adangel added a commit to adangel/pmd that referenced this issue Feb 10, 2023
@adangel
[doc] Update release notes (pmd#4120) [skip ci]
81b08f2
adangel added a commit to adangel/pmd that referenced this issue Feb 17, 2023
@adangel
[core] Require explicit language versions (pmd#4120)
6c95689
adangel added a commit to adangel/pmd that referenced this issue Feb 17, 2023
@adangel
Add explicit language versions (pmd#4120)
d7d2ad5
adangel added a commit to adangel/pmd that referenced this issue Feb 17, 2023
@adangel
[doc] Update release notes (pmd#4120) [skip ci]
8aaef01
@oowekyala oowekyala mentioned this issue Feb 18, 2023
Merged
5 tasks
@adangel
Copy link
Member

adangel commented Mar 2, 2023

This is fixed for PMD 7 via #4387

@adangel adangel closed this as completed Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
an:enhancement An improvement on existing features / rules
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

[core] Explicitly name all language versions adangel/pmd
3 participants
@jsotuyod @adangel @oowekyala