feat: add import detection (APP-77) #27

yaronskaya · 2017-09-07T17:25:42Z

No description provided.

astansler

Add some description of PR next time. Describe what you did and your intention on merging to master or just to keeping an updates on WIP.
Possibly regexs not working and can be optimized, I include fix in comments. Also, we should make tests in next PR.
I will implement empty methods and integrate those classes.

astansler · 2017-09-08T09:17:14Z

src/main/kotlin/app/extractors/CExtractor.kt

+    override fun extractImports(fileContent: List<String>): List<String> {
+        val libraries = mutableSetOf<String>()
+
+        val quoteRegex = Regex("#import\\s+\"(\\w+)[/\\w+]*\\.\\w+\"")


Did you test it? Probably shouldn't work.
Also, you can use only one regex with OR: #import\s+[\"|<](\w+)[/\w+]*.\w+[\"|>]

Fix double slash everywhere.
updates: Double slash ok here, but better to use triple quotes.

#include maybe?

astansler · 2017-09-08T09:22:32Z

src/main/kotlin/app/extractors/PhpExtractor.kt

+    override fun extractImports(fileContent: List<String>): List<String> {
+        val libraries = mutableSetOf<String>()
+
+        val regex = Regex("use\\s+(\\w+)[\\\\\\w+]*")


require, require_once?

include, include once?

astansler · 2017-09-08T09:25:14Z

src/main/kotlin/app/extractors/CsharpExtractor.kt

+    override fun extractImports(fileContent: List<String>): List<String> {
+        val libraries = mutableSetOf<String>()
+
+        val csLibraries = File("data/libraries/cs_libraries.txt")


This should be definitely optimized cuz everytime extractImports called a file will be read. IO operations take a lot of time. I'll do it.

astansler · 2017-09-08T09:26:05Z

src/main/kotlin/app/extractors/JavaExtractor.kt

+    override fun extractImports(fileContent: List<String>): List<String> {
+        val libraries = mutableSetOf<String>()
+
+        val javaLibraries = File("data/libraries/java_libraries.txt")


In some Extractors you using list of imports, in some not, why?

It depends. In js, there are sometimes no import lines, that's why we tokenize all lines and see what tokens are in the import list. In this case we should catch things like ((com|ru).)?organization.library.modules.* and also skip imports of inner files.

So you gonna use this libraries files in a classifier for every language?

astansler · 2017-09-08T09:26:58Z

src/main/kotlin/app/extractors/ObjectiveCExtractor.kt

+    override fun extractImports(fileContent: List<String>): List<String> {
+        val libraries = mutableSetOf<String>()
+
+        val importQuotesRegex = Regex("#import\\s+\"(\\w+)[/\\w+]*\\.\\w+\"")


astansler · 2017-09-08T09:27:48Z

src/main/kotlin/app/extractors/CsharpExtractor.kt

+import app.model.DiffFile
+import java.io.File
+
+class CsharpExtractor : ExtractorInterface {


CSharpExtractor

astansler · 2017-09-08T09:29:52Z

src/main/kotlin/app/extractors/CExtractor.kt

+
+class CExtractor : ExtractorInterface {
+    override fun extract(files: List<DiffFile>): List<CommitStats> {
+        TODO("not implemented")


This mean that exception will be thrown if this method will be called. Not a good idea to merge such things to master.

yandzee · 2017-09-08T10:25:54Z

src/main/kotlin/app/extractors/CExtractor.kt

+    override fun extractImports(fileContent: List<String>): List<String> {
+        val libraries = mutableSetOf<String>()
+
+        val quoteRegex = Regex("#import\\s+\"(\\w+)[/\\w+]*\\.\\w+\"")


#include maybe?

yandzee · 2017-09-08T10:27:54Z

src/main/kotlin/app/extractors/CppExtractor.kt

+    override fun extractImports(fileContent: List<String>): List<String> {
+        val libraries = mutableSetOf<String>()
+
+        val quoteRegex = Regex("#import\\s+\"(\\w+)[/\\w+]*\\.\\w+\"")


yandzee · 2017-09-08T10:32:24Z

src/main/kotlin/app/extractors/JavaExtractor.kt

+        val regex = Regex("import\\s+(\\w+[.\\w+]*)")
+        fileContent.forEach {
+            val res = regex.find(it)
+            if (res != null) {


if (res == null) return – inverse it to achieve lesser amount of nesting

yandzee · 2017-09-08T10:34:02Z

src/main/kotlin/app/extractors/PhpExtractor.kt

+    override fun extractImports(fileContent: List<String>): List<String> {
+        val libraries = mutableSetOf<String>()
+
+        val regex = Regex("use\\s+(\\w+)[\\\\\\w+]*")


include, include once?

sergey48k

approved by see comments

sergey48k · 2017-09-08T18:25:12Z

src/main/kotlin/app/extractors/ExtractorInterface.kt

+        files.map { file ->
+            file.old.imports = extractImports(file.old.content)
+            file.new.imports = extractImports(file.new.content)
+            file


Last line is return value.

sergey48k · 2017-09-08T18:29:09Z

src/main/kotlin/app/extractors/Extractor.kt

        val TYPE_LANGUAGE = 1
        val TYPE_KEYWORD = 2
-
        val SEPARATOR = ">"


is this used at all? maybe it can be removed?

Used in JavaExtractor for example to separate language from tech (or keywords, etc). "Java>Keyword".

sergey48k · 2017-09-10T04:03:05Z

src/main/kotlin/app/model/DiffEdit.kt


 package app.model

 import org.eclipse.jgit.diff.Edit


copyright/author

* feat: add import detection(APP-77) * feat: integrate import detection * feat: add data for libraries, fix php regex matching

feat: add import detection(APP-77)

bed1dd7

yaronskaya requested review from astansler and sergey48k September 7, 2017 17:25

astansler requested changes Sep 8, 2017

View reviewed changes

astansler assigned astansler and yaronskaya Sep 8, 2017

astansler changed the title ~~Add import detection(APP-77)~~ feat: add import detection (APP-77) Sep 8, 2017

yandzee requested changes Sep 8, 2017

View reviewed changes

yaronskaya and others added 3 commits September 8, 2017 16:25

fix pr

75c8a19

feat: integrate import detection

f1d977f

add data/libraries/, fix php regex matching

a45ca2c

sergey48k approved these changes Sep 10, 2017

View reviewed changes

astansler approved these changes Sep 11, 2017

View reviewed changes

yandzee approved these changes Sep 11, 2017

View reviewed changes

astansler merged commit 03dec51 into master Sep 11, 2017

astansler deleted the APP-77 branch September 11, 2017 10:03

asurkov pushed a commit that referenced this pull request Sep 11, 2017

feat: add import detection (APP-77) (#27)

c4cfa8d

* feat: add import detection(APP-77) * feat: integrate import detection * feat: add data for libraries, fix php regex matching

feat: add import detection (APP-77) #27

feat: add import detection (APP-77) #27

Uh oh!

Conversation

yaronskaya commented Sep 7, 2017

Uh oh!

astansler left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astansler Sep 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergey48k left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

astansler left a comment •

edited

Loading

astansler Sep 8, 2017 •

edited

Loading