Bug 401290 - Regression on import of Java classes with methods using varargs
Summary: Regression on import of Java classes with methods using varargs
Status: RESOLVED FIXED
Alias: None
Product: umbrello
Classification: Applications
Component: importer (show other bugs)
Version: Git
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Umbrello Development Group
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-21 21:19 UTC by Oliver Kellogg
Modified: 2018-11-26 10:28 UTC (History)
1 user (show)

See Also:
Latest Commit: https://commits.kde.org/umbrello/930ea0c87624cf7ea479cfb3e8e47ca9ea86f0f7
Version Fixed In: 2.26.90 (KDE Applications 18.11.90)

Attachments
Java classes that have methods with variable arguments (1.19 KB, application/gzip)
2018-11-21 21:19 UTC, Oliver Kellogg 		Details
Patch against git master umbrello/codeimport/javaimport.cpp @ be3b42f (1.13 KB, patch)
2018-11-21 22:14 UTC, Oliver Kellogg 		Details
Testcase for checking parameter datatype (259 bytes, application/gzip)
2018-11-26 10:24 UTC, Ralf Habacker 		Details
View All Add an attachment

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Kellogg 2018-11-21 21:19:02 UTC 
Created attachment 116456 [details]
Java classes that have methods with variable arguments

SUMMARY
Regression on import of Java classes with methods using varargs

STEPS TO REPRODUCE
1. cd to your preferred directory for source code to be imported
2. unpack the attachment: tar xvzf java_methods_with_varargs.tar.gz
3. in umbrello, select Code -> Active Language -> Java
4. in umbrello, select Code -> Import from Directory...
5. in the "Select directory to import" dialog, navigate to the top
   level directory ("com") created by unpacking the attachment and press OK

OBSERVED RESULT
1. In the course of the import, several times a dialog will pop up:
   > An object with the name Byte...
   > already exists in the Logical View.
   > Please enter a new name:
   > [new_class]
   > dito
   > [new_class_1]
2. When import is finished, a class "Byte..." will appear directly in the
   Logical View, along with the additional artifact classes (in this case,
   new_class and new_class_1)

EXPECTED RESULT
In version 2.21.70, no such dialog appeared, and
no unwanted artifacts were created.
The type "Byte..." appeared in the Datatypes folder.

I am using 2.26.3 with KF5 18.08.3 and Qt 5.11.
Comment 1 Oliver Kellogg 2018-11-21 22:14:49 UTC 
Created attachment 116457 [details]
Patch against git master umbrello/codeimport/javaimport.cpp @ be3b42f

This is a preliminary patch to get rid of the dialogs popping up during import which annoy when the code contains many methods with variable arguments.
Comment 2 Ralf Habacker 2018-11-22 08:49:03 UTC 
Thanks for reporting this issue. 

2. When import is finished, a class "Byte..." will appear directly in the
   Logical View,

According to https://cgit.kde.org/umbrello.git/tree/umbrello/codeimport/import_utils.cpp?h=Applications/18.12#n5370c, method parameter types are forced into the logical view folder. That the type was stored in the data type folder with version 2.21.70 is probably due to the fact that the folder assignment was overwritten elsewhere and corrected in the meantime.
Comment 3 Ralf Habacker 2018-11-22 09:27:05 UTC 
> A class "Byte..." will appear

https://stackoverflow.com/questions/3158730/java-3-dots-in-parameters says: 

"Important Note: The argument(s) passed in this way is always an array - even if there's just one. Make sure you treat it that way in the method body."

Letting the importer map this to Byte[] let this type be portable to other languages.
Comment 4 Oliver Kellogg 2018-11-22 15:23:19 UTC 
> Letting the importer map this to Byte[] let this type be portable to other
> languages.

Nice idea.
Would you like a patch?
Comment 5 Ralf Habacker 2018-11-22 17:26:59 UTC 
(In reply to Oliver Kellogg from comment #4)
> > Letting the importer map this to Byte[] let this type be portable to other
> > languages.
> 
> Nice idea.
> Would you like a patch?

Sure, see https://community.kde.org/Get_Involved/development#Submit_your_patch for submitting details.
Comment 6 Oliver Kellogg 2018-11-24 16:25:55 UTC 
Okay, see https://phabricator.kde.org/D17108
Comment 7 Ralf Habacker 2018-11-26 10:24:55 UTC 
Created attachment 116504 [details]
Testcase for checking parameter datatype

With the mentioned patch applied, the initial issue showing the 'is already present' dialog is fixed. The appended test case checks the other case, which is having a parameter datatype not being a basic type.

Check result: On importing the appended files the dialog box is not shown
Comment 8 Ralf Habacker 2018-11-26 10:28:39 UTC 
Git commit 930ea0c87624cf7ea479cfb3e8e47ca9ea86f0f7 by Ralf Habacker.
Committed on 26/11/2018 at 10:28.
Pushed by habacker into branch 'Applications/18.12'.

JavaImport: Convert Java specific notation '...' into a language independent array definition

In JavaImport::fillSource(), replace method vararg notation "..."
occurring in the input word by "[]".
FIXED-IN:2.26.90 (KDE Applications 18.11.90)
Reviewed By: habacker
Differential Revision: https://phabricator.kde.org/D17108

M  +10   -2    umbrello/codeimport/javaimport.cpp

https://commits.kde.org/umbrello/930ea0c87624cf7ea479cfb3e8e47ca9ea86f0f7