rquinio / l10n-maven-plugin

:abcd: Maven plugin to validate localization resources in Java properties files
MIT License
3 stars 1 forks source link

Support for String.format parameters #1

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Create a string with parameters to be replaced by String.format(...) calls.  
These parameters are in the format "%1", "%2", etc as documented here: 
http://docs.oracle.com/javase/1.5.0/docs/api/java/util/Formatter.html

2.  Create localized versions of the string with variations in the number or 
index of the parameters.

3. Run either mvn target.

What is the expected output? What do you see instead?
It would be great if these parameters were validated just like MessageFormat 
parameters - but they are not.

What version of the product are you using? On what operating system?
1.5

Please provide any additional information below.
First, thanks for open-sourcing this.  It's good stuff!

Second, it appears that this could be supported by changing the 
CAPTURE_PARAMETERS_REGEXP value in the ParametricCoherenceValidator class. Of 
course, the string format is referenced in several places in the documentation 
(site and javadoc), which would also require changes.

Original issue reported on code.google.com by patrick....@gmail.com on 29 Mar 2013 at 9:39

GoogleCodeExporter commented 9 years ago
Glad you found this plugin useful !

Do you mix C-style formatting (%1) and MessageFormat ({1}) in the same 
properties file ?

It sounds feasible to extract and compare the indexed parameters matching %(i)$ 
and ignore non indexed ones.

Today some MessageFormat parametric replacement (using integers) is performed 
before HTML and URL validation,  for cases where "{i,type}" would be invalid, 
for instance in the context of an URL/href. It may not be easy to support that 
for java.util.Formatter, which seems more strict.

Original comment by romain.q...@gmail.com on 7 Apr 2013 at 1:28

GoogleCodeExporter commented 9 years ago
In my current projects, we're only C-style formatting consistently -- we don't 
mix C-style and MessageFormat-style in a single properties file.  I can imagine 
doing so, but can't imagine doing so in good conscience!

Original comment by patrick....@gmail.com on 8 Apr 2013 at 10:12

GoogleCodeExporter commented 9 years ago

Original comment by romain.q...@gmail.com on 15 Nov 2013 at 12:00

GoogleCodeExporter commented 9 years ago
Indeed that would be a serious mess to mix both ^^, don't think it makes sense 
supporting it.

So I've added a plugin configuration parameter ("formatter") to switch between 
"messageFormat" (default) and "C-style" format.

Only explicitly indexed args are supported here as I use a simple %([0-9]+)$ 
regex.

Formatter is used by:
*Parametric coherence validator
*C-style parametric resource validator (but it does nothing, no single quotes 
nightmare with c-style)
*URL, HTML and js validator to get rid of format syntax that can interfere with 
validation (not done for C-style as it was unsafe, %f breaks if passing an 
integer arg for instance)

Original comment by romain.q...@gmail.com on 15 Nov 2013 at 3:25

GoogleCodeExporter commented 9 years ago
Can you test with version 1.7 ? 
Let me know if you get some false positives, my testing was limited to 
resources I invented myself as I don't have any project with C-style properties 
formatting.

Original comment by romain.q...@gmail.com on 15 Nov 2013 at 4:26

GoogleCodeExporter commented 9 years ago
Hi Romain -- Awesome -- thanks for working on this.  I tried v1.7 against our 
latest properties, and it pointed out quite a few things that I needed to fix.  
Once I got most everything cleaned up, I did some additional experimentation.  
I noticed two things I thought I'd bring to your attention.

When I run the l10n:validate goal, it does a great job of flagging 
discrepancies in the parameter count or names between the the localized files 
(e.g. between "myprops_en.properties" and "myprops_es.properties"), but doesn't 
flag differences between the localizedproperties and the default property (e.g. 
"myprops.properties", "myprops_en.properties").  I'm not sure if this is 
intentional or not.

When I run the l10n:report goal, it fails on a NullPointerException:

org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal 
com.googlecode.l10n-maven-plugin:l10n-maven-plugin:1.7:report (default-cli) on 
project silverpop-resource-bundle: Execution default-cli of goal 
com.googlecode.l10n-maven-plugin:l10n-maven-plugin:1.7:report failed
....
...
...
Caused by: java.lang.NullPointerException
        at java.io.File.<init>(File.java:222)
        at org.apache.maven.reporting.AbstractMavenReport.execute(AbstractMavenReport.java:73)
        at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo(DefaultBuildPluginManager.java:106)
        ... 20 more

Thanks again for doing this -- I really appreciate it (and it's helped me out 
already!).

Thanks,
Pat.

Original comment by patrick....@gmail.com on 15 Nov 2013 at 7:28

GoogleCodeExporter commented 9 years ago
Hi Patrick,

You're welcome, the design of that Formatter interface to know more about the 
Formatter was quite funny ^^

1) Actually there is no specific exclusion of root bundle for that check. There 
is a little logic to minimize the number of 1 vs 1 warnings raised, by assuming 
the most frequent is the good one. 
Does renaming root myprops.properties to myprops_zz.properties, changes 
anything ?

2) That would be getOutputDirectory() that is null. Plugin takes it from maven 
property ${project.reporting.outputDirectory}.

If you do mvn test -X you can check at the beginning of plugin execution the 
value being passed:
[DEBUG] Configuring mojo 
'com.googlecode.l10n-maven-plugin:l10n-maven-plugin:1.7-SNAPSHOT:validate' -->
...
[DEBUG]   (s) reportsDir = 
C:\Users\Romain\git\sample-project\target\l10n-reports
...
Also which Maven and JDK version are you using ?

3) I've just spotted that the new "formatter" parameter is not forwarded by 
ReportMojo to ValidateMojo, will have to fix that.

Cheers,
Romain

Original comment by romain.q...@gmail.com on 16 Nov 2013 at 4:30

GoogleCodeExporter commented 9 years ago
Sorry for the terribly slow response on this.  Yes, you're right that if I
change the root props to another locale, the discrepancies are still not
flagged.  That makes sense as you've explained it.

When I run "mvn test -X", I don't see the NPE.  The properties are set as
follows:
[DEBUG] Configuring mojo
'com.googlecode.l10n-maven-plugin:l10n-maven-plugin:1.7:validate' with
basic configurator -->
[DEBUG]   (s) formatter = C-style
[DEBUG]   (s) ignoreFailure = false
[DEBUG]   (s) jsDoubleQuoted = true
[DEBUG]   (s) propertyDir =
C:\Users\patrick.conant\Work\silverpop-resource-bundle\src\main\resources
[DEBUG]   (s) reportsDir =
C:\Users\patrick.conant\Work\silverpop-resource-bundle\target\l10n-reports
[DEBUG]   (s) xhtmlSchema =
C:\Users\patrick.conant\Work\silverpop-resource-bundle\xhtml1-transitional.xsd

When I run "mvn l10n:report -X", the properties are set as follows:

[DEBUG] Configuring mojo
'com.googlecode.l10n-maven-plugin:l10n-maven-plugin:1.7:report' with basic
configurator -->
[DEBUG]   (s) formatter = C-style
[DEBUG]   (s) jsDoubleQuoted = true
[DEBUG]   (s) propertyDir =
C:\Users\patrick.conant\Work\silverpop-resource-bundle\src\main\resources
[DEBUG] -- end configuration --

Original comment by patrick....@gmail.com on 25 Nov 2013 at 6:42